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

Update to napi-rs v2 #120

Merged
merged 7 commits into from
Apr 7, 2023
Merged

Update to napi-rs v2 #120

merged 7 commits into from
Apr 7, 2023

Conversation

devongovett
Copy link
Member

This updates the node bindings to use napi-rs v2, with the new derive macro which really simplifies generating classes and methods a lot. Also bumps Rust to the 2021 edition.

@devongovett
Copy link
Member Author

This seems to be causing segfaults occasionally in my testing, and I don't know why... Might be a bug in napi-rs.

* thread #14, stop reason = EXC_BAD_ACCESS (code=1, address=0x30)
  * frame #0: 0x000000012a447af8 index.darwin-arm64.node`_rjem_je_extent_heap_remove + 328
    frame #1: 0x000000012a449734 index.darwin-arm64.node`extents_remove_locked + 244
    frame #2: 0x000000012a4483e0 index.darwin-arm64.node`extent_recycle + 472
    frame #3: 0x000000012a4481fc index.darwin-arm64.node`_rjem_je_extents_alloc + 44
    frame #4: 0x000000012a434168 index.darwin-arm64.node`arena_bin_malloc_hard + 312
    frame #5: 0x000000012a433e3c index.darwin-arm64.node`_rjem_je_arena_tcache_fill_small + 556
    frame #6: 0x000000012a45a628 index.darwin-arm64.node`_rjem_je_tcache_alloc_small_hard + 28
    frame #7: 0x000000012a4360cc index.darwin-arm64.node`_rjem_je_arena_ralloc + 1588
    frame #8: 0x000000012a42e890 index.darwin-arm64.node`_rjem_rallocx + 320
    frame #9: 0x000000012a3ec168 index.darwin-arm64.node`alloc::raw_vec::finish_grow::h1d5846dc1bf2fa94 + 80
    frame #10: 0x000000012a3f18e0 index.darwin-arm64.node`parcel_sourcemap_node::__napi_impl_helper__JsSourceMap__0::__napi__add_vlq_map::he681e7f1857f8ee1 + 3728

Copy link
Member

@DeMoorJasper DeMoorJasper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so much cleaner 🤯. If the segmentation fault is not an issue with this version it looks good to me code-wise.

Sorry for the late review, this PR got buried in gh notifications

@devongovett
Copy link
Member Author

Seg fault should be fixed in latest napi now

@devongovett devongovett merged commit 6ef1795 into master Apr 7, 2023
18 checks passed
@devongovett devongovett deleted the napi-v2 branch April 7, 2023 15:39
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

Successfully merging this pull request may close these issues.

None yet

2 participants