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.

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

Throwing inside a JS callback triggers a segfault #55

Closed
arcanis opened this issue Dec 18, 2016 · 7 comments
Closed

Throwing inside a JS callback triggers a segfault #55

arcanis opened this issue Dec 18, 2016 · 7 comments

Comments

@arcanis
Copy link
Contributor

arcanis commented Dec 18, 2016

Cf title. Apparently, throwing from inside a JS callback triggers a segfault:

/home/arcanis/ohui/node_modules/segfault-handler/build/Release/segfault-handler.node(+0x1b60)[0x7f8c073a1b60]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x113e0)[0x7f8c0eb113e0]
node(_ZNK2v85Value10Int32ValueEv+0xc)[0xa8c57c]
/home/arcanis/ohui/node_modules/@manaflair/text-layout/build/Debug/nbind.node(_ZN5nbind10cbFunction4callIcJRjEEENS_15TypeTransformerIT_NS_14PolicyListTypeIJEEEE4TypeEDpOT0_+0xe4)[0x7f8c075d1214]
/home/arcanis/ohui/node_modules/@manaflair/text-layout/build/Debug/nbind.node(_ZN10TextLayout6updateEjjj+0x3311)[0x7f8c075d0481]
/home/arcanis/ohui/node_modules/@manaflair/text-layout/build/Debug/nbind.node(_ZN10TextLayout5resetEv+0x2af)[0x7f8c075d0b7f]
/home/arcanis/ohui/node_modules/@manaflair/text-layout/build/Debug/nbind.node(_ZN5nbind15MethodSignatureIM10TextLayoutF5PatchvES1_NS_14PolicyListTypeIJEEES2_JEE4callERKN3Nan20FunctionCallbackInfoIN2v85ValueEEE+0x19e)[0x7f8c075c82fe]
/home/arcanis/ohui/node_modules/@manaflair/text-layout/build/Debug/nbind.node(+0x361f1)[0x7f8c075dc1f1]
node(_ZN2v88internal25FunctionCallbackArguments4CallEPFvRKNS_20FunctionCallbackInfoINS_5ValueEEEE+0x165)[0xa961d5]
node[0xb0917c]
node(_ZN2v88internal21Builtin_HandleApiCallEiPPNS0_6ObjectEPNS0_7IsolateE+0x1d5)[0xb096f5]
[0x103bbd0063a7]
@jjrv
Copy link
Member

jjrv commented Dec 19, 2016

Interesting point. I suppose it should throw a C++ exception instead of crashing. Not much else it can do, if the callback is for example supposed to return an integer and throws instead. There's no reasonable value indicating an error to pass to C++.

@parro-it
Copy link
Contributor

Would it be possible to emit an uncaughtexception on node process object?

@arcanis
Copy link
Contributor Author

arcanis commented Dec 19, 2016

Throwing an exception seems fair. I'm not sure if that's already implemented, but it would be nice to also catch C++ exceptions and relay them to the JS code. Something like (pseudocode):

try {
    this->callInner(... args);
} catch (std::exception const & exception) {
    ThrowException(Exception::TypeError(String::New(exception.what())));
}

That way, even if a callback throws, the exception will still be dispatched synchronously as one would expect, and could still be catched as usual on the JS side (which wouldn't be possible if nbind was simply emitting an uncaughtException event).

@parro-it
Copy link
Contributor

That way, even if a callback throws, the exception will still be dispatched synchronously as one would expect, and could still be catched as usual on the JS side (which wouldn't be possible if nbind was simply emitting an uncaughtException event).

This is true only if the callback stack contains some js stack frame upwards. Otherwise, there is no js place to catch the exception synchronously. Node js libraries usually emit the uncaughtException event when that happen.

@jjrv
Copy link
Member

jjrv commented Dec 19, 2016

Can you test if the current develop branch fixes your issue?

@jjrv
Copy link
Member

jjrv commented Dec 19, 2016

It was already catching C++ exceptions on Node.js and turning the message into an equivalent JavaScript exception. Throwing C++ exceptions doesn't work on asm.js, but there the JavaScript exception already blows through any C++ code on the stack.

Now JavaScript exceptions thrown by callbacks on Node.js throw a C++ cbException without any message. When it gets caught by the nbind wrapper that originally called into C++, the JavaScript exception from the callback continues bubbling up the JavaScript stack. I guess it will emit uncaughtException when necessary.

This will cause very annoying issues on asm.js however. Since Emscripten doesn't properly support exceptions, no C++ destructors will get called for any objects on the C++ stack.

@parro-it
Copy link
Contributor

I guess it will emit uncaughtException when necessary.

It should, I'll test it soon...

@jjrv jjrv closed this as completed in ff9fc57 Dec 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants