src,test: fix and test Environments sharing an isolate by codebytere · Pull Request #66239 · nodejs/node

codebytere · 2026-09-23T12:17:32Z

embedding.md lets several Environments share an IsolateData, and with it an isolate and event loop, but no test builds that setup. The crashes fixed in it over the past month were all found by embedders.

This adds a cctest that does: it creates the isolate, CppHeap and contexts the way an embedder would, starts a few Environments that keep timers running, and then frees, stops and inspects them one at a time while checking that the others keep working. It runs once with a shared IsolateData and once with one per Environment. Reverting any of the recent fixes makes it fail.

Writing it turned up three small issues, fixed in separate commits:

The src: fix FreeEnvironment() breaking JS in sibling Environments #65977 fix didn't cover Environments with their own IsolateData; its counter is now per thread.
An Environment without an inspector threw a plain string from node:inspector; it now throws ERR_INSPECTOR_NOT_AVAILABLE.
Freeing an IsolateData before its Environments now fails a CHECK instead of leaving a dangling pointer.

It also corrects the FreeEnvironment() notes in embedding.md and node.h, which said JavaScript is disallowed on the whole isolate; only the Environment being freed loses it.

Disclosure: the code, tests, docs and this description were written by Claude Code, directed and reviewed by @codebytere.

nodejs-github-bot · 2026-09-23T12:17:38Z

Review requested:

@nodejs/inspector

codecov · 2026-09-23T14:52:36Z

Codecov Report

❌ Patch coverage is 91.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.37%. Comparing base (6dfe4eb) to head (65b3b9e).
⚠️ Report is 165 commits behind head on main.

Files with missing lines	Patch %	Lines
src/env.cc	85.71%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #66239      +/-   ##
==========================================
+ Coverage   90.28%   90.37%   +0.09%     
==========================================
  Files         789      790       +1     
  Lines      272878   273858     +980     
  Branches    52109    52395     +286     
==========================================
+ Hits       246363   247502    +1139     
+ Misses      16979    16872     -107     
+ Partials     9536     9484      -52

Files with missing lines	Coverage Δ
src/api/callback.cc	`83.25% <100.00%> (+0.47%)`	⬆️
src/env.h	`97.33% <100.00%> (+0.07%)`	⬆️
src/inspector_agent.cc	`83.07% <100.00%> (+1.33%)`	⬆️
src/node.h	`91.66% <ø> (ø)`
src/node_internals.h	`80.35% <ø> (ø)`
src/env.cc	`82.12% <85.71%> (+<0.01%)`	⬆️

... and 87 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The FreeEnvironment() fix for sibling Environments keeps the depth of nested Environment::CleanupHandles() calls on the IsolateData, so that InternalCallbackScope can re-allow JavaScript for sibling Environments while one of them is being freed. Environments that each have their own IsolateData on the same isolate and loop never see that counter and still fail with "illegal access". Environments that share a loop share a thread, so keep the depth in a thread_local instead. Refs: nodejs#65977 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

…out one An Environment created with kNoCreateInspector threw a bare string from inspector.Session#connect(), inspector.open() and the other Agent entry points, so callers could not tell the condition apart by error code. Use the ERR_INSPECTOR_NOT_AVAILABLE code that connectToMainThread() already throws for the same situation. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

FreeIsolateData() while an Environment created from it is still alive left that Environment with a dangling pointer and failed later in unrelated code. Count the Environments using an IsolateData and CHECK in its destructor that none are left. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

joyeecheung · 2026-09-24T23:37:21Z

hmm, I am not quite sure if this is something we are ready to support, IIRC we have a some places that simply assume "on thread, one isolate, one Environment" so making "multiple Environments sharing the same isolate" means these code now have to use something more sophisticated than thread_locals to retrieve states that are really per-Environment. I think some mechanism needs to be provided to organize these better before we can officially call it supported?

codebytere · 2026-09-25T13:19:50Z

@joyeecheung fair - i went through the thread_locals in src/ and it looks like two of them hold per-Environment state:

the root cert store in crypto_context.cc: tls.setDefaultCACertificates() in one Environment changes it for the others
quic_alloc_state in quic/bindingdata.cc: binding points at the last BindingData created on the thread, so one Environment's QUIC session frees against another's counter

The rest are re-entrancy guards and debug counters that are fine per thread.

Would something like this cover it?

a typed slot for native state on Environment (e.g. env->GetOrCreateNativeState<T>()), destroyed in FreeEnvironment()
callbacks reach it through their user_data or Environment::GetCurrent(isolate) instead of a thread_local
a lint that rejects new thread_locals in src/ unless they're marked as intentionally per-thread

i'll drop the docs commit so this PR doesn't call the setup supported, and can open the slot as its own PR first?

With the FreeEnvironment() fix for sibling Environments and the handle cleanup depth tracked per thread, only the Environment being freed loses JavaScript while FreeEnvironment() runs the shared loop. Callbacks of the other Environments on that loop run their JavaScript as usual. Update embedding.md and the comment in node.h, which still describe JavaScript as disallowed on the whole isolate. Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

legendecas · 2026-09-25T13:21:18Z

btw the PR title looks like this is a test only change.

nodejs-github-bot · 2026-09-25T13:25:09Z

CI: https://ci.nodejs.org/job/node-test-pull-request/77896/

codebytere · 2026-09-28T09:27:59Z

@legendecas @jasnell PTAL

nodejs-github-bot · 2026-09-28T09:39:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78031/

nodejs-github-bot · 2026-09-28T12:53:12Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78041/

nodejs-github-bot · 2026-09-28T15:03:16Z

CI: https://ci.nodejs.org/job/node-test-pull-request/78043/

nodejs-github-bot · 2026-09-29T13:03:03Z

Commit Queue failed

This pull request has multiple commits, but no landing policy was selected.

Add commit-queue-squash PRs the Commit Queue should land as one squashed commit. to land it as one commit, or commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. to land the commits separately.

The pull request was removed from the Commit Queue and labeled commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. . After resolving the failure, remove that label and add commit-queue PRs queued for automated landing through the Commit Queue. to retry.

Full Commit Queue output

�[36m⠋�[39m Loading data for nodejs/node/pull/66239
�[36m⠋�[39m Loading data for nodejs/node/pull/66239
�[36m⠋�[39m Getting collaborator contacts from README of nodejs/node
�[36m⠋�[39m Getting PR from nodejs/node/pull/66239
�[36m⠋�[39m Getting reviews from nodejs/node/pull/66239
�[36m⠋�[39m Getting comments from nodejs/node/pull/66239
�[36m⠋�[39m Getting commits from nodejs/node/pull/66239
✔  Done loading data for nodejs/node/pull/66239
----------------------------------- PR info ------------------------------------
Title      src,test: fix and test Environments sharing an isolate (#66239)
Author     Shelley Vohr <shelley.vohr@gmail.com> (@codebytere)
Branch     codebytere:test-multi-env-shared-isolate -> nodejs:main
Labels     c++, test, lib / src, embedding, author ready, needs-ci, commit-queue
Commits    5
 - src: track handle cleanup per thread, not per IsolateData
 - inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment with…
 - src: assert that an IsolateData outlives its Environments
 - test: cover Environments sharing an embedder-owned isolate
 - doc: update FreeEnvironment() notes for sibling Environments
Committers 1
 - Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 23 Sep 2026 12:17:32 GMT
   ✔  Approvals: 3
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/66239#pullrequestreview-5336633999
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/66239#pullrequestreview-5318124219
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/66239#pullrequestreview-5339164888
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-09-28T15:03:16Z: https://ci.nodejs.org/job/node-test-pull-request/78043/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78043/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78043/
�[36m⠙�[39m Querying API for job/node-test-pull-request/78043/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
�[36m⠹�[39m Bringing origin/main up to date...
�[36m⠹�[39m Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
�[36m⠸�[39m Downloading patch for 66239
�[36m⠸�[39m Downloading patch for 66239
From https://github.com/nodejs/node
 * branch                  refs/pull/66239/merge -> FETCH_HEAD
✔  Fetched commits as 471fe813bb38..65b3b9efdbb2
--------------------------------------------------------------------------------
Auto-merging src/api/callback.cc
Auto-merging src/env.cc
Auto-merging src/env.h
[main 7ac7e9887e] src: track handle cleanup per thread, not per IsolateData
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Wed Sep 23 12:14:17 2026 +0000
 4 files changed, 10 insertions(+), 9 deletions(-)
Auto-merging src/inspector_agent.cc
[main 6e42df01bc] inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment without one
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 2 insertions(+), 9 deletions(-)
Auto-merging src/env.cc
Auto-merging src/env.h
[main 83ad65c680] src: assert that an IsolateData outlives its Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 2 files changed, 10 insertions(+), 1 deletion(-)
[main 1c850a2cf9] test: cover Environments sharing an embedder-owned isolate
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 458 insertions(+)
 create mode 100644 test/cctest/test_environment_shared_isolate.cc
Auto-merging doc/api/embedding.md
Auto-merging src/node.h
[main 17580bf736] doc: update FreeEnvironment() notes for sibling Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:58:41 2026 +0000
 2 files changed, 6 insertions(+), 9 deletions(-)
   ✔  Patches applied
There are 5 commits in the PR. Attempting autorebase.
(node:467) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/10)
Executing: git node land --amend --yes
   ⚠  Found Refs: https://github.com/nodejs/node/pull/65977, skipping..
--------------------------------- New Message ----------------------------------
src: track handle cleanup per thread, not per IsolateData

The FreeEnvironment() fix for sibling Environments keeps the depth of
nested Environment::CleanupHandles() calls on the IsolateData, so that
InternalCallbackScope can re-allow JavaScript for sibling Environments
while one of them is being freed. Environments that each have their own
IsolateData on the same isolate and loop never see that counter and
still fail with "illegal access". Environments that share a loop share a
thread, so keep the depth in a thread_local instead.

Refs: https://github.com/nodejs/node/pull/65977
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD 09e6ca8a69] src: track handle cleanup per thread, not per IsolateData
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Wed Sep 23 12:14:17 2026 +0000
 4 files changed, 10 insertions(+), 9 deletions(-)
Rebasing (3/10)
Rebasing (4/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment without one

An Environment created with kNoCreateInspector threw a bare string from
inspector.Session#connect(), inspector.open() and the other Agent entry
points, so callers could not tell the condition apart by error code. Use
the ERR_INSPECTOR_NOT_AVAILABLE code that connectToMainThread() already
throws for the same situation.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD 52f7107cc3] inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment without one
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 2 insertions(+), 9 deletions(-)
Rebasing (5/10)
Rebasing (6/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
src: assert that an IsolateData outlives its Environments

FreeIsolateData() while an Environment created from it is still alive
left that Environment with a dangling pointer and failed later in
unrelated code. Count the Environments using an IsolateData and CHECK
in its destructor that none are left.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD a3e6baa30f] src: assert that an IsolateData outlives its Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 2 files changed, 10 insertions(+), 1 deletion(-)
Rebasing (7/10)
Rebasing (8/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: cover Environments sharing an embedder-owned isolate

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD 2a04ddac65] test: cover Environments sharing an embedder-owned isolate
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 458 insertions(+)
 create mode 100644 test/cctest/test_environment_shared_isolate.cc
Rebasing (9/10)
Rebasing (10/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: update FreeEnvironment() notes for sibling Environments

With the FreeEnvironment() fix for sibling Environments and the handle
cleanup depth tracked per thread, only the Environment being freed
loses JavaScript while FreeEnvironment() runs the shared loop. Callbacks
of the other Environments on that loop run their JavaScript as usual.
Update embedding.md and the comment in node.h, which still describe
JavaScript as disallowed on the whole isolate.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD 0a14b0f993] doc: update FreeEnvironment() notes for sibling Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:58:41 2026 +0000
 2 files changed, 6 insertions(+), 9 deletions(-)
Successfully rebased and updated refs/heads/main.
--------------------------------------------------------------------------------
   ℹ  Add `commit-queue-squash` label to land the PR as one commit, or `commit-queue-rebase` to land as separate commits.

View workflow run

nodejs-github-bot · 2026-09-29T19:38:11Z

Commit Queue failed

   ⚠  Found Refs: https://github.com/nodejs/node/pull/65977, skipping..
     ⚠  0:50     Title should be <= 50 columns.            title-length
  ✖  3b83c774be616e63a20ab1457a72c3c22adf4b93
     ✖  0:72     Title must be <= 72 columns.              title-length
     ⚠  0:50     Title should be <= 50 columns.            title-length
     ⚠  0:50     Title should be <= 50 columns.            title-length
     ⚠  0:50     Title should be <= 50 columns.            title-length

The pull request was removed from the Commit Queue and labeled commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. . After resolving the failure, remove that label and add commit-queue PRs queued for automated landing through the Commit Queue. to retry.

Full Commit Queue output

�[36m⠋�[39m Loading data for nodejs/node/pull/66239
�[36m⠋�[39m Loading data for nodejs/node/pull/66239
�[36m⠋�[39m Getting collaborator contacts from README of nodejs/node
�[36m⠋�[39m Getting PR from nodejs/node/pull/66239
�[36m⠋�[39m Getting reviews from nodejs/node/pull/66239
�[36m⠋�[39m Getting comments from nodejs/node/pull/66239
�[36m⠋�[39m Getting commits from nodejs/node/pull/66239
✔  Done loading data for nodejs/node/pull/66239
----------------------------------- PR info ------------------------------------
Title      src,test: fix and test Environments sharing an isolate (#66239)
Author     Shelley Vohr <shelley.vohr@gmail.com> (@codebytere)
Branch     codebytere:test-multi-env-shared-isolate -> nodejs:main
Labels     c++, test, lib / src, embedding, author ready, needs-ci, commit-queue, commit-queue-rebase
Commits    5
 - src: track handle cleanup per thread, not per IsolateData
 - inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment with…
 - src: assert that an IsolateData outlives its Environments
 - test: cover Environments sharing an embedder-owned isolate
 - doc: update FreeEnvironment() notes for sibling Environments
Committers 1
 - Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Wed, 23 Sep 2026 12:17:32 GMT
   ✔  Approvals: 3
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/66239#pullrequestreview-5336633999
   ✔  - Chengzhong Wu (@legendecas) (TSC): https://github.com/nodejs/node/pull/66239#pullrequestreview-5318124219
   ✔  - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/66239#pullrequestreview-5339164888
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-09-29T13:03:03Z: https://ci.nodejs.org/job/node-test-pull-request/78043/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78043/
�[36m⠙�[39m Querying data for job/node-test-pull-request/78043/
�[36m⠙�[39m Querying API for job/node-test-pull-request/78043/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
�[36m⠹�[39m Bringing origin/main up to date...
�[36m⠹�[39m Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
�[36m⠸�[39m Downloading patch for 66239
�[36m⠸�[39m Downloading patch for 66239
From https://github.com/nodejs/node
 * branch                  refs/pull/66239/merge -> FETCH_HEAD
✔  Fetched commits as c5b7a06d9b7c..65b3b9efdbb2
--------------------------------------------------------------------------------
Auto-merging src/api/callback.cc
Auto-merging src/env.cc
Auto-merging src/env.h
[main 9147db320c] src: track handle cleanup per thread, not per IsolateData
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Wed Sep 23 12:14:17 2026 +0000
 4 files changed, 10 insertions(+), 9 deletions(-)
Auto-merging src/inspector_agent.cc
[main 22114f4386] inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment without one
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 2 insertions(+), 9 deletions(-)
Auto-merging src/env.cc
Auto-merging src/env.h
[main ad1e381002] src: assert that an IsolateData outlives its Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 2 files changed, 10 insertions(+), 1 deletion(-)
[main 4585aa8baf] test: cover Environments sharing an embedder-owned isolate
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 458 insertions(+)
 create mode 100644 test/cctest/test_environment_shared_isolate.cc
Auto-merging doc/api/embedding.md
Auto-merging src/node.h
[main 75a0e8d063] doc: update FreeEnvironment() notes for sibling Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:58:41 2026 +0000
 2 files changed, 6 insertions(+), 9 deletions(-)
   ✔  Patches applied
There are 5 commits in the PR. Attempting autorebase.
(node:495) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/10)
Executing: git node land --amend --yes
   ⚠  Found Refs: https://github.com/nodejs/node/pull/65977, skipping..
--------------------------------- New Message ----------------------------------
src: track handle cleanup per thread, not per IsolateData

The FreeEnvironment() fix for sibling Environments keeps the depth of
nested Environment::CleanupHandles() calls on the IsolateData, so that
InternalCallbackScope can re-allow JavaScript for sibling Environments
while one of them is being freed. Environments that each have their own
IsolateData on the same isolate and loop never see that counter and
still fail with "illegal access". Environments that share a loop share a
thread, so keep the depth in a thread_local instead.

Refs: https://github.com/nodejs/node/pull/65977
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD 3aadd9ff0e] src: track handle cleanup per thread, not per IsolateData
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Wed Sep 23 12:14:17 2026 +0000
 4 files changed, 10 insertions(+), 9 deletions(-)
Rebasing (3/10)
Rebasing (4/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment without one

An Environment created with kNoCreateInspector threw a bare string from
inspector.Session#connect(), inspector.open() and the other Agent entry
points, so callers could not tell the condition apart by error code. Use
the ERR_INSPECTOR_NOT_AVAILABLE code that connectToMainThread() already
throws for the same situation.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD 3b83c774be] inspector: throw ERR_INSPECTOR_NOT_AVAILABLE from an Environment without one
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 2 insertions(+), 9 deletions(-)
Rebasing (5/10)
Rebasing (6/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
src: assert that an IsolateData outlives its Environments

FreeIsolateData() while an Environment created from it is still alive
left that Environment with a dangling pointer and failed later in
unrelated code. Count the Environments using an IsolateData and CHECK
in its destructor that none are left.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD ad938cc709] src: assert that an IsolateData outlives its Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 2 files changed, 10 insertions(+), 1 deletion(-)
Rebasing (7/10)
Rebasing (8/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: cover Environments sharing an embedder-owned isolate

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD d447a72afa] test: cover Environments sharing an embedder-owned isolate
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:48:46 2026 +0000
 1 file changed, 458 insertions(+)
 create mode 100644 test/cctest/test_environment_shared_isolate.cc
Rebasing (9/10)
Rebasing (10/10)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
doc: update FreeEnvironment() notes for sibling Environments

With the FreeEnvironment() fix for sibling Environments and the handle
cleanup depth tracked per thread, only the Environment being freed
loses JavaScript while FreeEnvironment() runs the shared loop. Callbacks
of the other Environments on that loop run their JavaScript as usual.
Update embedding.md and the comment in node.h, which still describe
JavaScript as disallowed on the whole isolate.

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/66239
Refs: https://github.com/nodejs/node/pull/65977
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
[detached HEAD 57bf66cefc] doc: update FreeEnvironment() notes for sibling Environments
 Author: Shelley Vohr <shelley.vohr@gmail.com>
 Date: Tue Sep 22 13:58:41 2026 +0000
 2 files changed, 6 insertions(+), 9 deletions(-)
Successfully rebased and updated refs/heads/main.
--------------------------------------------------------------------------------
  ✔  3aadd9ff0ec6b500aab0f7f999490011e6ddf94d
     ✔  0:0      no Assisted-by metadata                   assisted-by-is-trailer
     ✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✔  0:0      metadata is at end of message             metadata-end
     ✔  11:8     PR-URL is valid.                          pr-url
     ✔  0:0      reviewers are valid                       reviewers
     ✔  0:0      has valid Signed-off-by                   signed-off-by
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ⚠  0:50     Title should be <= 50 columns.            title-length
  ✖  3b83c774be616e63a20ab1457a72c3c22adf4b93
     ✔  0:0      no Assisted-by metadata                   assisted-by-is-trailer
     ✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✔  0:0      metadata is at end of message             metadata-end
     ✔  8:8      PR-URL is valid.                          pr-url
     ✔  0:0      reviewers are valid                       reviewers
     ✔  0:0      has valid Signed-off-by                   signed-off-by
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ✖  0:72     Title must be <= 72 columns.              title-length
  ✔  ad938cc7096c02561a59ffe886cb6765383cb666
     ✔  0:0      no Assisted-by metadata                   assisted-by-is-trailer
     ✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✔  0:0      metadata is at end of message             metadata-end
     ✔  7:8      PR-URL is valid.                          pr-url
     ✔  0:0      reviewers are valid                       reviewers
     ✔  0:0      has valid Signed-off-by                   signed-off-by
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ⚠  0:50     Title should be <= 50 columns.            title-length
  ✔  d447a72afa6385d1e48f33139f58f3d2b3599683
     ✔  0:0      no Assisted-by metadata                   assisted-by-is-trailer
     ✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✔  0:0      metadata is at end of message             metadata-end
     ✔  2:8      PR-URL is valid.                          pr-url
     ✔  0:0      reviewers are valid                       reviewers
     ✔  0:0      has valid Signed-off-by                   signed-off-by
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ⚠  0:50     Title should be <= 50 columns.            title-length
  ✔  57bf66cefca88315e8700de6dfe7236dfb9897bf
     ✔  0:0      no Assisted-by metadata                   assisted-by-is-trailer
     ✔  0:0      no Co-authored-by metadata                co-authored-by-is-trailer
     ✔  0:0      skipping fixes-url                        fixes-url
     ✔  0:0      blank line after title                    line-after-title
     ✔  0:0      line-lengths are valid                    line-length
     ✔  0:0      metadata is at end of message             metadata-end
     ✔  9:8      PR-URL is valid.                          pr-url
     ✔  0:0      reviewers are valid                       reviewers
     ✔  0:0      has valid Signed-off-by                   signed-off-by
     ✔  0:0      valid subsystems                          subsystem
     ✔  0:0      Title is formatted correctly.             title-format
     ⚠  0:50     Title should be <= 50 columns.            title-length
--------------------------------------------------------------------------------
   ℹ  Please fix the commit message and try again.
Please manually ammend the commit message, by running
`git commit --amend`
Once commit message is fixed, finish the landing command running
`git node land --continue`

View workflow run

codebytere added c++ Issues and PRs that require attention from people who are familiar with C++. test Issues and PRs related to Node.js core tests and test infrastructure. embedding Issues and PRs related to embedding Node.js in another project. labels Sep 23, 2026

nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 23, 2026

codebytere marked this pull request as ready for review September 23, 2026 12:59

codebytere requested review from jasnell, joyeecheung and theanarkh September 23, 2026 13:04

jasnell approved these changes Sep 23, 2026

View reviewed changes

codebytere added 4 commits September 23, 2026 15:39

test: cover Environments sharing an embedder-owned isolate

c171538

Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

codebytere force-pushed the test-multi-env-shared-isolate branch from f55fe32 to df2ae81 Compare September 23, 2026 15:41

codebytere added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 24, 2026

legendecas approved these changes Sep 25, 2026

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 25, 2026

codebytere changed the title ~~test: cover Environments sharing an embedder-owned isolate~~ src,test: fix and test Environments sharing an isolate Sep 25, 2026

panva added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Sep 25, 2026

codebytere force-pushed the test-multi-env-shared-isolate branch from df2ae81 to 65b3b9e Compare September 25, 2026 19:47

codebytere added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 25, 2026

codebytere requested a review from legendecas September 26, 2026 10:15

codebytere requested a review from jasnell September 26, 2026 10:15

codebytere mentioned this pull request Sep 26, 2026

src: keep per-Environment state out of thread_locals #66313

Closed

jasnell approved these changes Sep 28, 2026

View reviewed changes

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 28, 2026

addaleax approved these changes Sep 28, 2026

View reviewed changes

codebytere added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 29, 2026

nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Sep 29, 2026

Navigation Menu

Uh oh!

src,test: fix and test Environments sharing an isolate - #66239

src,test: fix and test Environments sharing an isolate#66239
codebytere wants to merge 5 commits into
nodejs:mainfrom
codebytere:test-multi-env-shared-isolate

codebytere commented Sep 23, 2026 •

edited

Loading

Uh oh!

nodejs-github-bot commented Sep 23, 2026

Uh oh!

codecov Bot commented Sep 23, 2026 •

edited

Loading

Uh oh!

joyeecheung commented Sep 24, 2026

Uh oh!

codebytere commented Sep 25, 2026 •

edited

Loading

Uh oh!

legendecas commented Sep 25, 2026

Uh oh!

nodejs-github-bot commented Sep 25, 2026

Uh oh!

codebytere commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 29, 2026

Uh oh!

nodejs-github-bot commented Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

codebytere commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nodejs-github-bot commented Sep 23, 2026

Uh oh!

codecov Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

joyeecheung commented Sep 24, 2026

Uh oh!

codebytere commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

legendecas commented Sep 25, 2026

Uh oh!

nodejs-github-bot commented Sep 25, 2026

Uh oh!

codebytere commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 28, 2026

Uh oh!

nodejs-github-bot commented Sep 29, 2026

Commit Queue failed

Uh oh!

nodejs-github-bot commented Sep 29, 2026

Commit Queue failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codebytere commented Sep 23, 2026 •

edited

Loading

codecov Bot commented Sep 23, 2026 •

edited

Loading

codebytere commented Sep 25, 2026 •

edited

Loading