Skip to content

malloc error on shutdown #53

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

Closed
samizdatco opened this issue Jul 28, 2020 · 5 comments · Fixed by #55 or #56
Closed

malloc error on shutdown #53

samizdatco opened this issue Jul 28, 2020 · 5 comments · Fixed by #55 or #56

Comments

@samizdatco
Copy link

When running the class-simple example from the current repo all the tests pass but then the script crashes at the end (presumably when the bound object is deallocated):

$ make test
nj-cli build
   Compiling nj-example-class-simple v0.1.0 (/.../node-bindgen/examples/class-simple)
    Finished dev [unoptimized + debuginfo] target(s) in 0.89s
node test.js
class simple test succeed
node(33705,0x10c5295c0) malloc: *** error for object 0x104400160: pointer being freed was not allocated
node(33705,0x10c5295c0) malloc: *** set a breakpoint in malloc_error_break to debug
make: *** [test] Abort trap: 6
version
macOS 10.14.6
node 14.5.0
rustc 1.45.0
@sehz
Copy link
Collaborator

sehz commented Jul 28, 2020

Thanks for reporting the crash. I can reproduce in my environment (Catalina, node v16) but not in CI. Strange

@sehz
Copy link
Collaborator

sehz commented Jul 28, 2020

It fails on linux (4.14, x86) as well.

make[1]: *** [test] Segmentation fault

@Ryanmtate
Copy link
Contributor

version
macOS 10.15.5 Beta
node v14.5.0+
rustc 1.45.0-nightly

I was able to reproduce this issue in node v14.5.0 - v14.7.0 (latest). I was not able to reproduce this issue before node v14.5.0. In the short-term, @samizdatco try downgrading to v14.4.0. Until then, I will investigate further.

RUST_LOG=debug make test
nj-cli build
node test.js
12:52:47.212 DEBUG nj_core::module > initializing modules
12:52:47.212 DEBUG nj_core::module > invoking register callback
12:52:47.213 DEBUG nj_core::basic  > defining class: MyObject with 0 properties
12:52:47.213 DEBUG nj_core::class  > Class constructor called: "nj_example_class_simple::MyObject"
12:52:47.213 DEBUG nj_core::class  > invoked as constructor
class simple test succeed
12:52:47.220 DEBUG nj_core::class  > my object finalize
12:52:47.221 DEBUG nj_core::class  > my object finalize
node(10298,0x1058dcdc0) malloc: *** error for object 0x105405930: pointer being freed was not allocated
node(10298,0x1058dcdc0) malloc: *** set a breakpoint in malloc_error_break to debug
make: *** [test] Abort trap: 6

@Ryanmtate
Copy link
Contributor

I might have found a solution after reading through the napi_wrap documentation.

#55 adds a delete_reference implementation to JsEnv, which calls napi_delete_reference.

See notes at the bottom of the napi_wrap documentation:

Caution: The optional returned reference (if obtained) should be deleted via napi_delete_reference ONLY in response to the > finalize callback invocation. If it is deleted before then, then the finalize callback may never be invoked. Therefore, when > obtaining a reference a finalize callback is also required in order to enable correct disposal of the reference.

Here is the resulting run for the simple-class example. Note, it removes the duplicate call to js_finalize, which was attempting to deallocate an already freed pointer.

RUST_LOG=debug make test
nj-cli build
node test.js
09:13:28.638 DEBUG nj_core::module > initializing modules
09:13:28.639 DEBUG nj_core::module > invoking register callback
09:13:28.639 DEBUG nj_core::basic  > defining class: MyObject with 0 properties
09:13:28.640 DEBUG nj_core::class  > Class constructor called: "nj_example_class_simple::MyObject"
09:13:28.640 DEBUG nj_core::class  > invoked as constructor
09:13:28.640 DEBUG nj_core::class  > wrapped reference deleted
class simple test succeed
09:13:28.646 DEBUG nj_core::class  > my object finalize

Still new to N-API, so any feedback is much appreciated.

@sehz
Copy link
Collaborator

sehz commented Aug 1, 2020

Please try again by doing cargo update and re-build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment