Title: [225343] trunk/Source/WebCore
- Revision
- 225343
- Author
- [email protected]
- Date
- 2017-11-30 13:01:12 -0800 (Thu, 30 Nov 2017)
Log Message
Make WorkerThread lifetime much more predictable.
https://bugs.webkit.org/show_bug.cgi?id=180203
Reviewed by Chris Dumez.
No new tests (Fixes flakiness in existing and future tests).
The family of classes related to Workers has a complicated ownership model.
For Dedicated Workers, the WorkerThread object is owned by the WorkerMessagingProxy,
which manages its own lifetime. Additionally, other object(s) have raw C++ references
to it, and the expected lifetimes are described in comments scattered through a few files.
What it boils down to is that the "Worker" DOM object - which lives on the main thread -
is the key to the proper destruction of all of these objects.
For ServiceWorkers running in their own context process, there is no "Worker" on the main thread.
As a result, ServiceWorkers can get into a situation where their WorkerThread can be destroyed before
their ServiceWorkerGlobalScope is destroyed on the running background thread.
There's no reason to not have WorkerThread guarantee its own lifetime until its background thread
has actually completed.
* workers/WorkerThread.cpp:
(WebCore::WorkerThread::workerThread): Protect the WorkerThread object during the entire runtime
of the background thread itself, and release that protection on the main thread.
* workers/WorkerThread.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (225342 => 225343)
--- trunk/Source/WebCore/ChangeLog 2017-11-30 20:48:53 UTC (rev 225342)
+++ trunk/Source/WebCore/ChangeLog 2017-11-30 21:01:12 UTC (rev 225343)
@@ -1,3 +1,34 @@
+2017-11-30 Brady Eidson <[email protected]>
+
+ Make WorkerThread lifetime much more predictable.
+ https://bugs.webkit.org/show_bug.cgi?id=180203
+
+ Reviewed by Chris Dumez.
+
+ No new tests (Fixes flakiness in existing and future tests).
+
+ The family of classes related to Workers has a complicated ownership model.
+
+ For Dedicated Workers, the WorkerThread object is owned by the WorkerMessagingProxy,
+ which manages its own lifetime. Additionally, other object(s) have raw C++ references
+ to it, and the expected lifetimes are described in comments scattered through a few files.
+
+ What it boils down to is that the "Worker" DOM object - which lives on the main thread -
+ is the key to the proper destruction of all of these objects.
+
+ For ServiceWorkers running in their own context process, there is no "Worker" on the main thread.
+
+ As a result, ServiceWorkers can get into a situation where their WorkerThread can be destroyed before
+ their ServiceWorkerGlobalScope is destroyed on the running background thread.
+
+ There's no reason to not have WorkerThread guarantee its own lifetime until its background thread
+ has actually completed.
+
+ * workers/WorkerThread.cpp:
+ (WebCore::WorkerThread::workerThread): Protect the WorkerThread object during the entire runtime
+ of the background thread itself, and release that protection on the main thread.
+ * workers/WorkerThread.h:
+
2017-11-30 Chris Dumez <[email protected]>
Populate self.registration.installing/waiting/active inside service workers
Modified: trunk/Source/WebCore/workers/WorkerThread.cpp (225342 => 225343)
--- trunk/Source/WebCore/workers/WorkerThread.cpp 2017-11-30 20:48:53 UTC (rev 225342)
+++ trunk/Source/WebCore/workers/WorkerThread.cpp 2017-11-30 21:01:12 UTC (rev 225343)
@@ -149,6 +149,8 @@
void WorkerThread::workerThread()
{
+ auto protectedThis = makeRef(*this);
+
// Propagate the mainThread's fenv to workers.
#if PLATFORM(IOS)
FloatingPointEnvironment::singleton().propagateMainThreadEnvironment();
@@ -231,6 +233,9 @@
// Clean up WebCore::ThreadGlobalData before WTF::Thread goes away!
threadGlobalData().destroy();
+ // Send the last WorkerThread Ref to be Deref'ed on the main thread.
+ callOnMainThread([protectedThis = WTFMove(protectedThis)] { });
+
// The thread object may be already destroyed from notification now, don't try to access "this".
protector->detach();
}
Modified: trunk/Source/WebCore/workers/WorkerThread.h (225342 => 225343)
--- trunk/Source/WebCore/workers/WorkerThread.h 2017-11-30 20:48:53 UTC (rev 225342)
+++ trunk/Source/WebCore/workers/WorkerThread.h 2017-11-30 21:01:12 UTC (rev 225343)
@@ -59,7 +59,7 @@
struct WorkerThreadStartupData;
-class WorkerThread : public RefCounted<WorkerThread> {
+class WorkerThread : public ThreadSafeRefCounted<WorkerThread> {
public:
virtual ~WorkerThread();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
