Skip to content

fix: load native addon in worker_threads - #2

Merged
enochgroot merged 2 commits into
security/modernize-2.0.0from
fix/worker-threads
Sep 10, 2026
Merged

enochgroot merged 2 commits into
security/modernize-2.0.0from
fix/worker-threads

Conversation

@enochgroot

Copy link
Copy Markdown
Owner

Summary

Fixes msgpack/msgpack-node#60: requiring the native addon in the main thread then in a worker_threads Worker threw Error: Module did not self-register.

  • Register with NODE_MODULE_CONTEXT_AWARE instead of NODE_MODULE.
  • Keep the sbuffer pool, unpack.bytes_remaining, and cycle-detection key thread_local so workers do not race the main isolate.

Test plan

  • npm test — 57/57 pass (includes 4 new worker_threads tests)
  • Repro: require() in main, then in a Worker — no longer throws
  • Worker pack/unpack of {a:1, b: Buffer.from('hi')}
  • Cycle detection still throws inside a worker
  • Main-thread binding.bytesRemaining() is not clobbered by a worker unpack

NODE_MODULE is not context-aware, so requiring msgpack in the main
thread then in a Worker throws "Module did not self-register".

Register with NODE_MODULE_CONTEXT_AWARE and keep the sbuffer pool,
unpack remainder, and cycle-detection key thread_local so workers
do not race the main isolate.

Fixes msgpack#60
Address review on PR #2 for worker_threads (msgpack#60):

- Mark sbuf_pool thread_local so a worker pack cannot share the pool
- Register with NAN_MODULE_WORKER_ENABLED instead of NODE_MODULE_CONTEXT_AWARE
- Return non-pooled sbuffers to the pool so it actually fills
- Add concurrent pack (workers + main) and sequential pool-reuse tests
@enochgroot

Copy link
Copy Markdown
Owner Author

Review round 2 (f2236a0 on fix/worker-threads):

  • sbuf_pool is now static thread_local SbufPool sbuf_pool;
  • Module register is NAN_MODULE_WORKER_ENABLED(msgpackBinding, Init) (the NODE_MODULE_CONTEXT_AWARE function-pointer warning is gone)
  • ~PackBuffer offers every sbuffer back to the thread's pool unless it is full, so the pool actually fills
  • Concurrent pack test: 4 workers × 4000 round-trips overlapping the main thread; sequential 2000-pack pool-reuse test for CopyBuffer vs NewBuffer

npm test: 57 pass / 0 fail (Node 20.20.2 linux/arm64). Rebuild clean.

Does not open a PR against msgpack/msgpack-node.

@enochgroot
enochgroot merged commit 5578c8a into security/modernize-2.0.0 Sep 10, 2026
12 checks passed
@enochgroot enochgroot mentioned this pull request Sep 10, 2026
3 tasks
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.

1 participant