[v10.x] src: dispose of V8 platform in `process.exit()` by MylesBorins · Pull Request #26048 · nodejs/node

gireeshpunathil

BethGriggs pushed a commit that referenced this pull request

Feb 13, 2019
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 all
Worker threads and then the V8 platform instance helps with this
(although it might not be a full solution for all problems of
this kind).

Refs: #24403
Refs: #25007

Backport-PR-URL: #26048
PR-URL: #25061
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

BethGriggs

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 all
Worker threads and then the V8 platform instance helps with this
(although it might not be a full solution for all problems of
this kind).

Refs: nodejs#24403
Refs: nodejs#25007

PR-URL: nodejs#25061
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Node first calls `Isolate::Dispose`, then
`NodePlatform::UnregisterIsolate`.
This again calls `PerIsolatePlatformData::Shutdown`, which (before this
patch) called `FlushForegroundTasksInternal`, which might call
`RunForegroundTask` if it finds foreground tasks to be executed. This
will fail however, since `Isolate::GetCurrent` was already reset during
`Isolate::Dispose`.
Hence remove the check to `FlushForegroundTasksInternal` and add checks
instead that no more foreground tasks are scheduled.

Refs: v8#86

PR-URL: nodejs#25653
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

BethGriggs

gireeshpunathil

BethGriggs pushed a commit that referenced this pull request

Mar 28, 2019
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 all
Worker threads and then the V8 platform instance helps with this
(although it might not be a full solution for all problems of
this kind).

Refs: #24403
Refs: #25007

Backport-PR-URL: #26048
PR-URL: #25061
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

BethGriggs pushed a commit that referenced this pull request

Mar 28, 2019
Node first calls `Isolate::Dispose`, then
`NodePlatform::UnregisterIsolate`.
This again calls `PerIsolatePlatformData::Shutdown`, which (before this
patch) called `FlushForegroundTasksInternal`, which might call
`RunForegroundTask` if it finds foreground tasks to be executed. This
will fail however, since `Isolate::GetCurrent` was already reset during
`Isolate::Dispose`.
Hence remove the check to `FlushForegroundTasksInternal` and add checks
instead that no more foreground tasks are scheduled.

Refs: v8#86

Backport-PR-URL: #26048
PR-URL: #25653
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>