Fix segfaults when used with worker_threads (#195)

* fix: don't use static constructor field with global scope which results in a race condition between worker threads. * feat: bump node-addon-api to ^3.2.1 which adds instance data to env. * update ci * bump node version Co-authored-by: mariuspod <[email protected]>
cryptocoinjs · Dec 30, 2022 · 3482be9 · 3482be9
1 parent 4afabb4
commit 3482be9
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 26 deletions.
diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
@@ -15,28 +15,17 @@ jobs:
         os:
           - macos-latest
           - ubuntu-latest
-          - windows-latest
+          - windows-2019
     steps:
       - name: Fetch code
         uses: actions/checkout@v1
         with:
           submodules: true
 
-      - name: Install dependencies
-        run: yarn install --ignore-scripts
-
-      - name: Build addon
-        if: runner.os != 'Linux'
-        run: make build-addon
-
-      - name: Build addon
-        if: runner.os == 'Linux'
-        run: make build-addon-linux
-
       - name: Get minimal Node.js version from package.json (Linux & macOS)
         id: node-version-nix
         if: runner.os != 'Windows'
-        run: echo "::set-output name=version::$(node -p 'require("./package.json").engines.node.match(/(\d.*)$/)[0]')"
+        run: echo "::set-output name=version::$(node -p 'require("./package.json").engines.node.match(/(\d+)\..*$/)[1]')"
 
       - name: Use Node.js ${{ steps.node-version-nix.outputs.version }} (Linux & macOS)
         if: runner.os != 'Windows'
@@ -47,14 +36,25 @@ jobs:
       - name: Get minimal Node.js version from package.json (Windows)
         id: node-version-win
         if: runner.os == 'Windows'
-        run: echo "::set-output name=version::$(node -p 'require(\"./package.json\").engines.node.match(/(\d.*)$/)[0]')"
+        run: echo "::set-output name=version::$(node -p 'require(\"./package.json\").engines.node.match(/(\d+)\..*$/)[1]')"
 
       - name: Use Node.js ${{ steps.node-version-win.outputs.version }} (Windows)
         if: runner.os == 'Windows'
         uses: actions/setup-node@v1
         with:
           node-version: ${{ steps.node-version-win.outputs.version }}
 
+      - name: Install dependencies
+        run: yarn install --ignore-scripts
+
+      - name: Build addon
+        if: runner.os != 'Linux'
+        run: make build-addon
+
+      - name: Build addon
+        if: runner.os == 'Linux'
+        run: make build-addon-linux
+
       - name: Run tests for addon
         run: make test-tap
 

diff --git a/Makefile b/Makefile
@@ -9,10 +9,10 @@ prebuildify-cross = ./node_modules/.bin/prebuildify-cross
 # hack, otherwise GitHub Actions for Windows:
 #  '.' is not recognized as an internal or external command, operable program or batch file.
 build-addon:
-	$(prebuildify) --target node@10.0.0 --napi --strip && node -p "process.platform"
+	$(prebuildify) --target node@14.0.0 --napi --strip && node -p "process.platform"
 
 build-addon-linux:
-	$(prebuildify-cross) -i centos7-devtoolset7 -i alpine --target node@10.0.0 --napi --strip
+	$(prebuildify-cross) -i centos7-devtoolset7 -i alpine --target node@14.0.0 --napi --strip
 
 
 nyc = ./node_modules/.bin/nyc

diff --git a/README.md b/README.md
@@ -2,7 +2,7 @@
 
 This module provides native bindings to [bitcoin-core/secp256k1](https://github.com/bitcoin-core/secp256k1). In browser [elliptic](https://github.com/indutny/elliptic) will be used as fallback.
 
-Works on node version 10.0.0 or greater, because use [N-API](https://nodejs.org/api/n-api.html).
+Works on node version 14.0.0 or greater, because use [N-API](https://nodejs.org/api/n-api.html).
 
 ## Installation
 

diff --git a/binding.gyp b/binding.gyp
@@ -83,7 +83,7 @@
         '-fno-exceptions',
       ],
       'defines': [
-        'NAPI_VERSION=3',
+        'NAPI_VERSION=6',
       ],
       'xcode_settings': {
         'GCC_ENABLE_CPP_EXCEPTIONS': 'YES',

diff --git a/package.json b/package.json
@@ -33,7 +33,7 @@
   },
   "dependencies": {
     "elliptic": "^6.5.4",
-    "node-addon-api": "^2.0.0",
+    "node-addon-api": "^5.0.0",
     "node-gyp-build": "^4.2.0"
   },
   "devDependencies": {
@@ -48,7 +48,7 @@
     "yargs": "^15.0.2"
   },
   "engines": {
-    "node": ">=10.0.0"
+    "node": ">=14.0.0"
   },
   "gypfile": true
 }
diff --git a/src/secp256k1.cc b/src/secp256k1.cc
@@ -26,8 +26,7 @@
   } while (0)
 
 // Secp256k1
-Napi::FunctionReference Secp256k1Addon::constructor;
-unsigned int Secp256k1Addon::secp256k1_context_flags =
+const unsigned int Secp256k1Addon::secp256k1_context_flags =
     SECP256K1_CONTEXT_SIGN | SECP256K1_CONTEXT_VERIFY;
 
 Napi::Value Secp256k1Addon::Init(Napi::Env env) {
@@ -66,8 +65,9 @@ Napi::Value Secp256k1Addon::Init(Napi::Env env) {
           InstanceMethod("ecdh", &Secp256k1Addon::ECDH),
       });
 
-  constructor = Napi::Persistent(func);
-  constructor.SuppressDestruct();
+  Napi::FunctionReference* constructor = new Napi::FunctionReference();
+  *constructor = Napi::Persistent(func);
+  env.SetInstanceData<Napi::FunctionReference>(constructor);
 
   return func;
 }

diff --git a/src/secp256k1.h b/src/secp256k1.h
@@ -28,8 +28,7 @@ class Secp256k1Addon : public Napi::ObjectWrap<Secp256k1Addon> {
   };
 
  private:
-  static Napi::FunctionReference constructor;
-  static unsigned int secp256k1_context_flags;
+  static const unsigned int secp256k1_context_flags;
   const secp256k1_context* ctx_;
   ECDSASignData ecdsa_sign_data;
   ECDHData ecdh_data;