Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2849583004:
Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() (Closed)
|
Created:
3 years, 7 months ago by nhiroki Modified:
3 years, 7 months ago Reviewers:
tzik CC:
chromium-reviews, shimazu+worker_chromium.org, kinuko+worker_chromium.org, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, blink-worker-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionWorker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask()
When forcible termination is requested, WorkerThread::WillProcessTask() calls
PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler,
which in turn disposes of the task to be run. This results in crashes.
This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to
DidProcessTask() to fix the crashes. This could still cause a crash if the
termination request is issued between DidProcessTask() and WillProcessTask()
because in the case a task runs and may access an empty handle returned by V8
API. However, such a case would be rare because generally forcible termination
is scheduled as a delayed task(*) and DidProcessTask() can have a chance to
check IsForciblyTerminated() before it's fired.
(*) Forcible termination could synchronously be called in some path, but it'll
be removed (see issue 641846).
BUG=713914
Review-Url: https://codereview.chromium.org/2849583004
Cr-Commit-Position: refs/heads/master@{#468001}
Committed: https://chromium.googlesource.com/chromium/src/+/611639da9364de0f00eb0136523482a9b7b394b7
Patch Set 1 #
Messages
Total messages: 19 (15 generated)
The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() WorkerThread::WillProcessTask() may call PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from DidProcessTask() to WillProcessTask() to fix the crashes. BUG=713914 ========== to ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. BUG=713914 ==========
Patchset #1 (id:1) has been deleted
The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. BUG=713914 ========== to ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. This could cause another crash if the termination request is issued between DidProcessTask() and WillProcessTask() because in the case a task runs and may access an empty handle returned by V8 API. However, such a case would be rare because forcible termination is scheduled as a delayed task(*) and in the most of cases DidProcessTask() is called before it's fired. (*) Forcible termination could synchronously be called in some path, but it'll be removed (see issue 641846). BUG=713914 ==========
nhiroki@chromium.org changed reviewers: + tzik@chromium.org
PTAL, thanks!
Description was changed from ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. This could cause another crash if the termination request is issued between DidProcessTask() and WillProcessTask() because in the case a task runs and may access an empty handle returned by V8 API. However, such a case would be rare because forcible termination is scheduled as a delayed task(*) and in the most of cases DidProcessTask() is called before it's fired. (*) Forcible termination could synchronously be called in some path, but it'll be removed (see issue 641846). BUG=713914 ========== to ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. This could cause another crash if the termination request is issued between DidProcessTask() and WillProcessTask() because in the case a task runs and may access an empty handle returned by V8 API. However, such a case would be rare because generally forcible termination is scheduled as a delayed task(*) and DidProcessTask() can have a chance to check IsForciblyTerminated() before it's fired. (*) Forcible termination could synchronously be called in some path, but it'll be removed (see issue 641846). BUG=713914 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
Description was changed from ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. This could cause another crash if the termination request is issued between DidProcessTask() and WillProcessTask() because in the case a task runs and may access an empty handle returned by V8 API. However, such a case would be rare because generally forcible termination is scheduled as a delayed task(*) and DidProcessTask() can have a chance to check IsForciblyTerminated() before it's fired. (*) Forcible termination could synchronously be called in some path, but it'll be removed (see issue 641846). BUG=713914 ========== to ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. This could still cause a crash if the termination request is issued between DidProcessTask() and WillProcessTask() because in the case a task runs and may access an empty handle returned by V8 API. However, such a case would be rare because generally forcible termination is scheduled as a delayed task(*) and DidProcessTask() can have a chance to check IsForciblyTerminated() before it's fired. (*) Forcible termination could synchronously be called in some path, but it'll be removed (see issue 641846). BUG=713914 ==========
The CQ bit was checked by nhiroki@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493392487924730,
"parent_rev": "f305a94dd4908c4975db3444e6b80732f48dcb39", "commit_rev":
"611639da9364de0f00eb0136523482a9b7b394b7"}
Message was sent while issue was closed.
Description was changed from ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. This could still cause a crash if the termination request is issued between DidProcessTask() and WillProcessTask() because in the case a task runs and may access an empty handle returned by V8 API. However, such a case would be rare because generally forcible termination is scheduled as a delayed task(*) and DidProcessTask() can have a chance to check IsForciblyTerminated() before it's fired. (*) Forcible termination could synchronously be called in some path, but it'll be removed (see issue 641846). BUG=713914 ========== to ========== Worker: Stop disposing of the global scope scheduler in WorkerThread::WillProcessTask() When forcible termination is requested, WorkerThread::WillProcessTask() calls PrepareForShutdownOnWorkerThread() that disposes of the global scope scheduler, which in turn disposes of the task to be run. This results in crashes. This CL moves PrepareForShutdownOnWorkerThread() call from WillProcessTask() to DidProcessTask() to fix the crashes. This could still cause a crash if the termination request is issued between DidProcessTask() and WillProcessTask() because in the case a task runs and may access an empty handle returned by V8 API. However, such a case would be rare because generally forcible termination is scheduled as a delayed task(*) and DidProcessTask() can have a chance to check IsForciblyTerminated() before it's fired. (*) Forcible termination could synchronously be called in some path, but it'll be removed (see issue 641846). BUG=713914 Review-Url: https://codereview.chromium.org/2849583004 Cr-Commit-Position: refs/heads/master@{#468001} Committed: https://chromium.googlesource.com/chromium/src/+/611639da9364de0f00eb01365234... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:20001) as https://chromium.googlesource.com/chromium/src/+/611639da9364de0f00eb01365234... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
