Title: [120328] trunk/Source/WebCore
- Revision
- 120328
- Author
- [email protected]
- Date
- 2012-06-14 08:39:34 -0700 (Thu, 14 Jun 2012)
Log Message
Worker tear-down can re-enter JSC during GC finalization pt. 2
https://bugs.webkit.org/show_bug.cgi?id=88601
Reviewed by David Levin.
No new tests. Current regression tests are sufficient.
* workers/WorkerMessagingProxy.cpp:
(WebCore::WorkerMessagingProxy::WorkerMessagingProxy):
(WebCore::WorkerMessagingProxy::workerObjectDestroyed): We clear the m_workerObject here because
we don't want anybody else trying to send messages to the Worker now that it has been destroyed.
We also queue the asynchronous task for the various other cleanup that still needs to be done.
This allows us to avoid the problem of re-entrant JS code execution during GC.
(WebCore):
(WebCore::WorkerMessagingProxy::workerObjectDestroyedInternal): Here we set m_mayBeDestroyed to true.
This is the point after which deleting the WorkerMessagingProxy in workerContextDestroyedInternal()
is okay. It could happen during this function call if the worker thread has been shutdown already, or
it could be called later after we shut down the worker thread.
(WebCore::WorkerMessagingProxy::workerContextDestroyedInternal): We check m_mayBeDestroyed here
instead of checking m_workerObject. This change effectively orthogonalizes the roles that m_workerObject
was filling. Since we were eagerly clearing m_workerObject, but we wanted to asynchronously call
workerObjectDestroyed(), we needed to make sure we didn't accidentally try to delete the WorkerMessagingProxy
twice (once from destroying the Worker and once from destroying the WorkerContext). This boolean field
should fix that issue--we set it lazily like we wanted to do without being in danger of causing use-after-free
issues with m_workerObject.
* workers/WorkerMessagingProxy.h: Added the new field and function.
(WorkerMessagingProxy):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (120327 => 120328)
--- trunk/Source/WebCore/ChangeLog 2012-06-14 15:34:05 UTC (rev 120327)
+++ trunk/Source/WebCore/ChangeLog 2012-06-14 15:39:34 UTC (rev 120328)
@@ -1,3 +1,33 @@
+2012-06-13 Mark Hahnenberg <[email protected]>
+
+ Worker tear-down can re-enter JSC during GC finalization pt. 2
+ https://bugs.webkit.org/show_bug.cgi?id=88601
+
+ Reviewed by David Levin.
+
+ No new tests. Current regression tests are sufficient.
+
+ * workers/WorkerMessagingProxy.cpp:
+ (WebCore::WorkerMessagingProxy::WorkerMessagingProxy):
+ (WebCore::WorkerMessagingProxy::workerObjectDestroyed): We clear the m_workerObject here because
+ we don't want anybody else trying to send messages to the Worker now that it has been destroyed.
+ We also queue the asynchronous task for the various other cleanup that still needs to be done.
+ This allows us to avoid the problem of re-entrant JS code execution during GC.
+ (WebCore):
+ (WebCore::WorkerMessagingProxy::workerObjectDestroyedInternal): Here we set m_mayBeDestroyed to true.
+ This is the point after which deleting the WorkerMessagingProxy in workerContextDestroyedInternal()
+ is okay. It could happen during this function call if the worker thread has been shutdown already, or
+ it could be called later after we shut down the worker thread.
+ (WebCore::WorkerMessagingProxy::workerContextDestroyedInternal): We check m_mayBeDestroyed here
+ instead of checking m_workerObject. This change effectively orthogonalizes the roles that m_workerObject
+ was filling. Since we were eagerly clearing m_workerObject, but we wanted to asynchronously call
+ workerObjectDestroyed(), we needed to make sure we didn't accidentally try to delete the WorkerMessagingProxy
+ twice (once from destroying the Worker and once from destroying the WorkerContext). This boolean field
+ should fix that issue--we set it lazily like we wanted to do without being in danger of causing use-after-free
+ issues with m_workerObject.
+ * workers/WorkerMessagingProxy.h: Added the new field and function.
+ (WorkerMessagingProxy):
+
2012-06-14 Alexander Pavlov <[email protected]>
Web Inspector: Selector list start position is not extracted for style rules inside @media rules
Modified: trunk/Source/WebCore/workers/WorkerMessagingProxy.cpp (120327 => 120328)
--- trunk/Source/WebCore/workers/WorkerMessagingProxy.cpp 2012-06-14 15:34:05 UTC (rev 120327)
+++ trunk/Source/WebCore/workers/WorkerMessagingProxy.cpp 2012-06-14 15:39:34 UTC (rev 120328)
@@ -250,6 +250,7 @@
WorkerMessagingProxy::WorkerMessagingProxy(Worker* workerObject)
: m_scriptExecutionContext(workerObject->scriptExecutionContext())
, m_workerObject(workerObject)
+ , m_mayBeDestroyed(false)
, m_unconfirmedMessageCount(0)
, m_workerThreadHadPendingActivity(false)
, m_askedToTerminate(false)
@@ -354,10 +355,16 @@
void WorkerMessagingProxy::workerObjectDestroyed()
{
m_workerObject = 0;
- if (m_workerThread)
- terminateWorkerContext();
+ m_scriptExecutionContext->postTask(createCallbackTask(&workerObjectDestroyedInternal, AllowCrossThreadAccess(this)));
+}
+
+void WorkerMessagingProxy::workerObjectDestroyedInternal(ScriptExecutionContext*, WorkerMessagingProxy* proxy)
+{
+ proxy->m_mayBeDestroyed = true;
+ if (proxy->m_workerThread)
+ proxy->terminateWorkerContext();
else
- workerContextDestroyedInternal();
+ proxy->workerContextDestroyedInternal();
}
#if ENABLE(INSPECTOR)
@@ -438,7 +445,7 @@
InspectorInstrumentation::workerContextTerminated(m_scriptExecutionContext.get(), this);
- if (!m_workerObject)
+ if (m_mayBeDestroyed)
delete this;
}
Modified: trunk/Source/WebCore/workers/WorkerMessagingProxy.h (120327 => 120328)
--- trunk/Source/WebCore/workers/WorkerMessagingProxy.h 2012-06-14 15:34:05 UTC (rev 120327)
+++ trunk/Source/WebCore/workers/WorkerMessagingProxy.h 2012-06-14 15:39:34 UTC (rev 120328)
@@ -99,11 +99,13 @@
virtual ~WorkerMessagingProxy();
void workerContextDestroyedInternal();
+ static void workerObjectDestroyedInternal(ScriptExecutionContext*, WorkerMessagingProxy*);
void reportPendingActivityInternal(bool confirmingMessage, bool hasPendingActivity);
Worker* workerObject() const { return m_workerObject; }
RefPtr<ScriptExecutionContext> m_scriptExecutionContext;
Worker* m_workerObject;
+ bool m_mayBeDestroyed;
RefPtr<DedicatedWorkerThread> m_workerThread;
unsigned m_unconfirmedMessageCount; // Unconfirmed messages from worker object to worker thread.
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
