src: dispose of V8 platform in process.exit() #24828
Closed
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
Calling `process.exit()` calls the C `exit()` function, which in turn calls the destructors of static C++ objects. This can lead to race conditions with other concurrently executing threads; disposing of the V8 platform instance helps with this (although it might not be a full solution for all problems of this kind). Fixes: nodejs#24403
nodejs-github-bot added the c++ 
richardlau approved these changes Dec 4, 2018
joyeecheung approved these changes Dec 4, 2018
Member Author
| Beginning to wonder though … does this interfere with Workers? They might be attempting to access the platform while we are shutting it down here, right? :/ |
Member
| Isn't the proper solution to join all threads first? |
Member
| If I'm not mistaken, it looks like this doesn't fix test-cli-syntax. https://ci.nodejs.org/job/node-test-commit-linux/23744/nodes=centos7-64-gcc6/testReport/ is a CI run for this PR. assert.js:86 throw new AssertionError(obj); ^ AssertionError [ERR_ASSERTION]: Expected values to be strictly equal: null !== 1 at common.mustCall (/home/iojs/build/workspace/node-test-commit-linux/nodes/centos7-64-gcc6/test/parallel/test-cli-syntax.js:97:14) at /home/iojs/build/workspace/node-test-commit-linux/nodes/centos7-64-gcc6/test/common/index.js:346:15 at ChildProcess.exithandler (child_process.js:301:5) at ChildProcess.emit (events.js:189:13) at maybeClose (internal/child_process.js:977:16) at Socket.stream.socket.on (internal/child_process.js:395:11) at Socket.emit (events.js:189:13) at Pipe._handle.close (net.js:612:12) |
Member Author
| Yeah, I’m closing this, will have to look into it a bit more… |
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 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.
Calling
process.exit()calls the Cexit()function, which in turncalls the destructors of static C++ objects. This can lead to race
conditions with other concurrently executing threads; disposing of the
V8 platform instance helps with this (although it might not be a full
solution for all problems of this kind).
Fixes: #24403
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes