neon-bindings / neon

Rust bindings for writing safe and fast native Node.js modules.
https://www.neon-bindings.com/
Apache License 2.0
8.04k stars 284 forks source link

Sigkill on load #911

Open netshade opened 2 years ago

netshade commented 2 years ago

When loading index.node module via a node repl on an M1 Mac, the process would immediately exit sometimes ( I did not find a good reason for why this occurred sometimes when building a node module ). I found this issue in the nodejs repo, and explicitly code signing index.node did in fact allow the node process to load the module and interact w/ it successfully.

Brooooooklyn commented 2 years ago

codesign --verbose -s - index.node

kjvalencik commented 2 years ago

+1 to what @Brooooooklyn said about resigning. You can also rm index.node prior to re-building. This is a bug (known limitation?) in Apple's code signing cache.

kjvalencik commented 2 years ago

I don't have an M1 mac to test with, but I've been curious if adding an unlink step to cargo-cp-artifact prior to copying the lib would fix the issue.

netshade commented 2 years ago

Like something here that would essentially say if arch == arm64 and platform == Darwin { recodesign } ? Or by saying unlink do you mean something else?

I'm happy to take a crack at it since I can test it.

kjvalencik commented 2 years ago

Probably something in the copyArtifact function. By unlink, I mean delete.

https://nodejs.org/api/fs.html#fspromisesunlinkpath

Brooooooklyn commented 2 years ago

https://github.com/napi-rs/napi-rs/pull/413/files unlink before copy is working

kjvalencik commented 2 years ago

Oh, yea, I remember investigating it previously. Deleting the file in cargo-cp-artifact first works well for cases where it's the final destination (and this should be added if you're interested @netshade); however, if the file is copied again by something like Webpack, the issue will remain.

Wherever the final copy is that's loaded, that copy needs to be deleted prior to copying.

In addition to the cargo-cp-artifact fix, this could benefit from some documentation for the issue and the workaround.

kjvalencik commented 2 years ago

Re-opening since this could benefit from some documentation describing the specific errors and messages for users that have the problem outside of cargo-cp-artifact's control.

netshade commented 2 years ago

Awesome to see the fix tho, @kjvalencik . Thanks for that :)