Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

FFI Crash with "DisallowJavascriptExecutionScope" when callback is called many times #22947

Closed
Leokuma opened this issue Mar 15, 2024 · 3 comments · Fixed by #24367
Closed

FFI Crash with "DisallowJavascriptExecutionScope" when callback is called many times #22947

Leokuma opened this issue Mar 15, 2024 · 3 comments · Fixed by #24367
Labels
FFI Related to Foreign Function Interface APIs fix available there is a fix available in a third party library that needs to be merged upstream Changes in upstream are required to solve these issues

Comments

@Leokuma
Copy link

Leokuma commented Mar 15, 2024

Deno crashes when a callback is called thousands of times by an external lib through FFI. I don't know if it's a bug or I'm doing something wrong. I've verified the same behavior in Windows 11 and Ubuntu 22. Example below.

lib.cpp

extern "C" {
  extern void call_cb(void (*callback)()) {
    callback();
  }
}

Compile with:

$ g   -fPIC -c lib.cpp;
$ g   --shared -o lib.so lib.o;

main.ts

const lib = Deno.dlopen(new URL("./lib.so", import.meta.url), {
  call_cb: {parameters: ["function"], result: "void"}
});

const cb = new Deno.UnsafeCallback({parameters: [], result: "void"}, () => {});

let i = 30000; // increase this if you don't see the crash
while (i--)
  lib.symbols.call_cb(cb.pointer);

Crash

$ deno run -A --unstable-ffi main.ts           


#
# Fatal error in , line 0
# Invoke in DisallowJavascriptExecutionScope
#
#
#
#FailureMessage Object: 00000000007E67F0
==== C stack trace ===============================

        napi_release_threadsafe_function [0x00007FF732FD7B7B 28028160]
        napi_release_threadsafe_function [0x00007FF732FD6BC7 28024140]
        napi_release_threadsafe_function [0x00007FF733033CCD 28405330]
        napi_release_threadsafe_function [0x00007FF73310E5C8 29300557]
        napi_release_threadsafe_function [0x00007FF73310D752 29296855]
        napi_release_threadsafe_function [0x00007FF732FC6033 27955640]
        napi_release_threadsafe_function [0x00007FF732FAB3B9 27845950]
        napi_release_threadsafe_function [0x00007FF731B06367 6198508]
        napi_release_threadsafe_function [0x00007FF731B0447C 6190593]
        CrashForExceptionInNonABICompliantCodeRange [0x00007FF73411B5DD 16362733]
        CrashForExceptionInNonABICompliantCodeRange [0x00007FF73411DDBE 16372942]
        call_cb [0x00007FFEADFF1392 18]
        (No symbol) [0x00007FF6B41432AB]

Deno version

deno 1.41.1 (release, x86_64-pc-windows-msvc)
v8 12.1.285.27
typescript 5.3.3
@aapoalas
Copy link
Collaborator

V8 forbids fast API calls from calling back into JS. You need to mark the symbol with callback: true to let V8 / Deno know that this function may call back into JS and thus should not use fast API.

@aapoalas aapoalas reopened this Mar 15, 2024
@aapoalas
Copy link
Collaborator

Reopened because it seems we're missing documentation about this in the runtime API manual page, and are actually guiding to cause callbacks to happen without this.

@Leokuma
Copy link
Author

Leokuma commented Mar 15, 2024

Thank you so much!

@littledivy littledivy added docs FFI Related to Foreign Function Interface APIs labels Mar 16, 2024
@littledivy littledivy mentioned this issue Jun 29, 2024
Merged
@littledivy littledivy added upstream Changes in upstream are required to solve these issues fix available there is a fix available in a third party library that needs to be merged and removed docs labels Jun 29, 2024
littledivy added a commit that referenced this issue Jul 1, 2024
@littledivy
BREAKING(unstable/ffi): remove callback reentrant flag (#24367)
cd4a007 
Closes #22947

This option is no longer needed as fast calls are now allowed to
re-enter the isolate
sbmsr pushed a commit to sbmsr/deno-1 that referenced this issue Jul 2, 2024
@littledivy @sbmsr
BREAKING(unstable/ffi): remove callback reentrant flag (denoland#24367)
f577116 
Closes denoland#22947

This option is no longer needed as fast calls are now allowed to
re-enter the isolate
zebreus pushed a commit to zebreus/deno that referenced this issue Jul 8, 2024
@littledivy @zebreus
BREAKING(unstable/ffi): remove callback reentrant flag (denoland#24367)
2d26c59 
Closes denoland#22947

This option is no longer needed as fast calls are now allowed to
re-enter the isolate
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FFI Related to Foreign Function Interface APIs fix available there is a fix available in a third party library that needs to be merged upstream Changes in upstream are required to solve these issues
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

BREAKING(unstable/ffi): remove callback reentrant flag littledivy/deno
3 participants
@Leokuma @aapoalas @littledivy