src: fix creating Isolates from addons#45885
Merged
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
daae938 broke addons which create their own `Isolate` instances, because enabling the shared-readonly-heap feature of V8 requires all snapshots used for different `Isolate`s to be identical. Usage of addons that do this has probably decreased quite a bit since Worker threads were introduced in Node.js, but it’s still a valid use case, and in any case the breakage was probably not intentional (although the referenced commit did require test changes because of this issue). This commit addresses this issue partially by caching the V8 snapshot parameters and ignoring ones passed in from users in `NewIsolate()` when this feature is enabled, and makes the `NodeMainInstance` snapshot-based isolate creation also re-use this code.
Collaborator
nodejs-github-bot commented Dec 16, 2022
Review requested:
|
nodejs-github-bot added c++ This was referenced Dec 16, 2022
github-actionsbot removed the request-ci Collaborator
nodejs-github-bot commented Dec 16, 2022
This was referenced Dec 17, 2022
jasnell approved these changes Dec 19, 2022
Collaborator
nodejs-github-bot commented Dec 19, 2022
Collaborator
nodejs-github-bot commented Dec 19, 2022
14 tasks
github-actionsbot removed the request-ci Collaborator
nodejs-github-bot commented Dec 20, 2022
Collaborator
nodejs-github-bot commented Dec 20, 2022
12 tasks
Collaborator
nodejs-github-bot commented Dec 21, 2022
addaleax added author ready This was referenced Dec 22, 2022
addaleax added commit-queue nodejs-github-bot removed the commit-queue Collaborator
nodejs-github-bot commented Dec 23, 2022
Landed in 43e09af |
This was referenced Dec 24, 2022
targos pushed a commit that referenced this pull request Jan 1, 2023
daae938 broke addons which create their own `Isolate` instances, because enabling the shared-readonly-heap feature of V8 requires all snapshots used for different `Isolate`s to be identical. Usage of addons that do this has probably decreased quite a bit since Worker threads were introduced in Node.js, but it’s still a valid use case, and in any case the breakage was probably not intentional (although the referenced commit did require test changes because of this issue). This commit addresses this issue partially by caching the V8 snapshot parameters and ignoring ones passed in from users in `NewIsolate()` when this feature is enabled, and makes the `NodeMainInstance` snapshot-based isolate creation also re-use this code. PR-URL: #45885 Reviewed-By: James M Snell <[email protected]>
Merged
RafaelGSS pushed a commit that referenced this pull request Jan 4, 2023
daae938 broke addons which create their own `Isolate` instances, because enabling the shared-readonly-heap feature of V8 requires all snapshots used for different `Isolate`s to be identical. Usage of addons that do this has probably decreased quite a bit since Worker threads were introduced in Node.js, but it’s still a valid use case, and in any case the breakage was probably not intentional (although the referenced commit did require test changes because of this issue). This commit addresses this issue partially by caching the V8 snapshot parameters and ignoring ones passed in from users in `NewIsolate()` when this feature is enabled, and makes the `NodeMainInstance` snapshot-based isolate creation also re-use this code. PR-URL: #45885 Reviewed-By: James M Snell <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Jan 5, 2023
daae938 broke addons which create their own `Isolate` instances, because enabling the shared-readonly-heap feature of V8 requires all snapshots used for different `Isolate`s to be identical. Usage of addons that do this has probably decreased quite a bit since Worker threads were introduced in Node.js, but it’s still a valid use case, and in any case the breakage was probably not intentional (although the referenced commit did require test changes because of this issue). This commit addresses this issue partially by caching the V8 snapshot parameters and ignoring ones passed in from users in `NewIsolate()` when this feature is enabled, and makes the `NodeMainInstance` snapshot-based isolate creation also re-use this code. PR-URL: #45885 Reviewed-By: James M Snell <[email protected]>
juanarbol pushed a commit that referenced this pull request Jan 26, 2023
daae938 broke addons which create their own `Isolate` instances, because enabling the shared-readonly-heap feature of V8 requires all snapshots used for different `Isolate`s to be identical. Usage of addons that do this has probably decreased quite a bit since Worker threads were introduced in Node.js, but it’s still a valid use case, and in any case the breakage was probably not intentional (although the referenced commit did require test changes because of this issue). This commit addresses this issue partially by caching the V8 snapshot parameters and ignoring ones passed in from users in `NewIsolate()` when this feature is enabled, and makes the `NodeMainInstance` snapshot-based isolate creation also re-use this code. PR-URL: #45885 Reviewed-By: James M Snell <[email protected]>
Merged
juanarbol pushed a commit that referenced this pull request Jan 31, 2023
daae938 broke addons which create their own `Isolate` instances, because enabling the shared-readonly-heap feature of V8 requires all snapshots used for different `Isolate`s to be identical. Usage of addons that do this has probably decreased quite a bit since Worker threads were introduced in Node.js, but it’s still a valid use case, and in any case the breakage was probably not intentional (although the referenced commit did require test changes because of this issue). This commit addresses this issue partially by caching the V8 snapshot parameters and ignoring ones passed in from users in `NewIsolate()` when this feature is enabled, and makes the `NodeMainInstance` snapshot-based isolate creation also re-use this code. PR-URL: #45885 Reviewed-By: James M Snell <[email protected]>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
daae938 broke addons which create their own
Isolateinstances, because enabling the shared-readonly-heap feature of V8 requires all snapshots used for differentIsolates to be identical. Usage of addons that do this has probably decreased quite a bit since Worker threads were introduced in Node.js, but it’s still a valid use case, and in any case the breakage was probably not intentional (although the referenced commit did require test changes because of this issue).This commit addresses this issue partially by caching the V8 snapshot parameters and ignoring ones passed in from users in
NewIsolate()when this feature is enabled, and makes theNodeMainInstancesnapshot-based isolate creation also re-use this code.