-
Notifications
You must be signed in to change notification settings - Fork 30.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
src: dispose of V8 platform in process.exit()
#24828
Conversation
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
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? :/ |
Isn't the proper solution to join all threads first? |
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) |
Yeah, I’m closing this, will have to look into it a bit more… |
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