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

Updates node-addon-api #79

Merged
merged 5 commits into from
Jan 17, 2024
Merged

Updates node-addon-api #79

merged 5 commits into from
Jan 17, 2024

Conversation

vdeturckheim
Copy link
Contributor

@simon-id , please add some tests when you have a way to reproduce the issue

@simon-id simon-id requested a review from a team as a code owner November 10, 2023 14:18
@simon-id
Copy link
Member

Potentially solves #82

Bump Node-API version to 6
change use of global constructor for DDWAFContext constructor to a thread safe solution
@uurien uurien changed the title Updates node-addon-api to the latest Updates node-addon-api Jan 16, 2024
@uurien
Copy link
Contributor

uurien commented Jan 16, 2024

This PR fixes the problems related with worker_threads and the waf. Added a test to check that it is working fine.
This PR also break the compatibility with node12, so next release should be a major release.

Here is a very short explanation about why this solves the problem:
nodejs/node-addon-api#711

Explanation: this example set the constructor static member of the class to a reference of the constructor function but if the addon is loaded multiple time (with worker_threads for example), the value is overwritten by a new reference in the new context. That makes this reference unusable to create new instances of this wrapped class from other C++ code and makes any code wanting to use this reference in another context crash.

@vdeturckheim
Copy link
Contributor Author

🚀 🚀 🚀
image

@vdeturckheim
Copy link
Contributor Author

image

package.json Show resolved Hide resolved
@uurien uurien merged commit 6a4000a into main Jan 17, 2024
35 checks passed
@uurien uurien deleted the multithreading branch January 17, 2024 11:03
@simon-id simon-id mentioned this pull request Jan 18, 2024
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.

3 participants