-
Notifications
You must be signed in to change notification settings - Fork 459
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
Signal SIGSEGV in v8::internal::GlobalHandles::Create(v8::internal::Object*) () #393
Comments
@legraphista This is something that shouldn’t be happening, no. Could you provide a way to reproduce this? |
Will do! I'll throw together a demo project that illustrates the issue. In the meantime, the same scenario also occasionally throws this stack trace: (gdb)
I have a hunch it might be from the |
@legraphista It’s hard to tell from the stack traces – this could be a bug in N-API, in V8 or in your code… My best guess would be that this is some use-after-free bug for |
Hi, as promised, i'm back with an example. In the example i also detail sort of a solution/workaround where i don't move data by storing into Float32Arrays but by passing External pointers.
|
The valgrind output is pretty clear about this being an use-after-free situation – not necessarily in your code, though. It sounds like the issue is something like this: After a GC run, one persistent handle finalizer callback (the one for the I am not sure what to do about this; it seems like an issue that can occur in very generic situations with |
A workaround that I've found is to call GC from javascript after each iteration (or a couple of), like here. I've found it to be stable (at least in the limited testing i did). |
Having looked at the issue I think the top commit in this branch might resolve the issue but I've not looked at testing on your code yet: https://github.com/mhdawson/io.js/tree/finalizer-order2 The main change is that if a request to delete a reference is made before the finalizer has run for the associated object it defers the delete until the finalizer runs. I think this makes sense for the case where we had a workaround in place for when a finalizer callback called delete on a reference and I'm hoping it also resolves the issue you were seeing. |
@legraphista could you try out that change and see if it resolves the issue for you? |
Just noticed I missed pushing the commit to the branch doing that now |
Branch updated. |
In respect, to |
@mhdawson That looks like it could this issue, yes. 👍 |
After some testing, I've come back with results:
Each configuration was tasted over multiple runs. Testing was done on:
|
I've thrown in macOS since in my original testing I haven't included it. To my surprise, with the same scenario as on the Linux environment, I cannot reproduce the crash. The stress test finished multiple times without a hitch. For both environments, versions 9, 10, and 11 were downloaded & installed with nvm. The v12 branch was compiled with If deemed necessary, I could compile v10 and v11 locally on the linux box, and see if the issue persists. |
@legraphista thanks, one more piece of data that would be great is v12.0.0-pre WITHOUT my change. If we see it fail there, then we'll be sure that it was my change that fixed it. |
Ah yes, of course, my bad.
|
@legraphista thanks. Will submit PR and ask V8 team members to comment as well. |
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393
This may be similar to nodejs/node#23999. |
PR to fix: nodejs/node#24494 |
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393 PR-URL: #24494 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393 PR-URL: #24494 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393 PR-URL: nodejs#24494 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393 Backport-PR-URL: nodejs#25572 PR-URL: nodejs#24494 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393 Backport-PR-URL: nodejs#25574 PR-URL: nodejs#24494 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393 Backport-PR-URL: #25574 PR-URL: #24494 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Crashes were reported during finalization due to the memory for a reference being deleted and the finalizer running after the deletion. This change ensures the deletion of the memory for the reference only occurs after the finalizer has run. Fixes: nodejs/node-addon-api#393 Backport-PR-URL: #25574 PR-URL: #24494 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Hi!
I've noticed some crashes sporadically occurring in v8 when calling the constructor of a class from the OnOK handler of
AsyncWorker
s. The crashes only seem to affect node 10.x (tested on 10.5, 10.13, 9.11.2). I'm runningnode-addon-api
1.6.0Stack trace: (gdb)
I've put together a list to the best of my knowledge resembling the code path from the stack trace:
frame 4: https://github.com/nodejs/node-addon-api/blob/master/napi-inl.h#L2824
frame 5: https://github.com/legraphista/darknet-binding/blob/089917035a5b188197a3f71b6f7bc2a87fa3604b/src/DarknetImage.cc#L37
frame 19: https://github.com/legraphista/darknet-binding/blob/089917035a5b188197a3f71b6f7bc2a87fa3604b/src/DarknetImage.h#L102
Has this happened to anyone else, or am I doing funky stuff i shouldn't be doing?
Thanks
The text was updated successfully, but these errors were encountered: