Home Original page

platform: Ensure no more foreground tasks after Deinit by backes · Pull Request #86 · v8/node

@backes 

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.

hashseed

approved these changes Jan 22, 2019

addaleax pushed a commit to addaleax/node that referenced this pull request

Jan 23, 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

addaleax pushed a commit to nodejs/node that referenced this pull request

Jan 27, 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

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

addaleax pushed a commit to nodejs/node that referenced this pull request

Jan 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

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

BethGriggs pushed a commit to MylesBorins/node that referenced this pull request

Mar 27, 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

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

BethGriggs pushed a commit to nodejs/node 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>