[webkit-changes] [215528] trunk/Source/WebCore

[commit-queue]

Title: [215528] trunk/Source/WebCore

Revision

215528

Author

commit-qu...@webkit.org

Date

2017-04-19 13:22:49 -0700 (Wed, 19 Apr 2017)

Log Message

ASAN Crash running LayoutTests/inspector/worker tests
https://bugs.webkit.org/show_bug.cgi?id=170967
<rdar://problem/31256437>

Patch by Joseph Pecoraro <pecor...@apple.com> on 2017-04-19
Reviewed by Alex Christensen.

* workers/WorkerMessagingProxy.h:
* workers/WorkerMessagingProxy.cpp:
(WebCore::WorkerMessagingProxy::WorkerMessagingProxy):
(WebCore::WorkerMessagingProxy::workerGlobalScopeDestroyedInternal):
Make the MessagingProxy thread safe ref counted. Since it used to
delete itself, turn this into a ref (implicit on construction)
and deref (replacing delete this).

(WebCore::WorkerMessagingProxy::postMessageToPageInspector):
When dispatching have the lambda implicitly ref/deref with the
lambda to keep the proxy alive while a lambda is queued.

Modified Paths

• trunk/Source/WebCore/ChangeLog
• trunk/Source/WebCore/workers/WorkerMessagingProxy.cpp
• trunk/Source/WebCore/workers/WorkerMessagingProxy.h

Diff

Modified: trunk/Source/WebCore/ChangeLog (215527 => 215528)

--- trunk/Source/WebCore/ChangeLog	2017-04-19 20:20:15 UTC (rev 215527)
+++ trunk/Source/WebCore/ChangeLog	2017-04-19 20:22:49 UTC (rev 215528)
@@ -1,3 +1,23 @@
+2017-04-19  Joseph Pecoraro  <pecor...@apple.com>
+
+        ASAN Crash running LayoutTests/inspector/worker tests
+        https://bugs.webkit.org/show_bug.cgi?id=170967
+        <rdar://problem/31256437>
+
+        Reviewed by Alex Christensen.
+
+        * workers/WorkerMessagingProxy.h:
+        * workers/WorkerMessagingProxy.cpp:
+        (WebCore::WorkerMessagingProxy::WorkerMessagingProxy):
+        (WebCore::WorkerMessagingProxy::workerGlobalScopeDestroyedInternal):
+        Make the MessagingProxy thread safe ref counted. Since it used to
+        delete itself, turn this into a ref (implicit on construction)
+        and deref (replacing delete this).
+
+        (WebCore::WorkerMessagingProxy::postMessageToPageInspector):
+        When dispatching have the lambda implicitly ref/deref with the
+        lambda to keep the proxy alive while a lambda is queued.
+
 2017-04-19  Brent Fulgham  <bfulg...@apple.com>
 
         [iOS, macOS] Guard against passing nullptr to vImagePremultiplyData

Modified: trunk/Source/WebCore/workers/WorkerMessagingProxy.cpp (215527 => 215528)

--- trunk/Source/WebCore/workers/WorkerMessagingProxy.cpp	2017-04-19 20:20:15 UTC (rev 215527)
+++ trunk/Source/WebCore/workers/WorkerMessagingProxy.cpp	2017-04-19 20:22:49 UTC (rev 215528)
@@ -58,6 +58,9 @@
 {
     ASSERT((is<Document>(*m_scriptExecutionContext) && isMainThread())
         || (is<WorkerGlobalScope>(*m_scriptExecutionContext) && currentThread() == downcast<WorkerGlobalScope>(*m_scriptExecutionContext).thread().threadID()));
+
+    // Nobody outside this class ref counts this object. The original ref
+    // is balanced by the deref in workerGlobalScopeDestroyedInternal.
 }
 
 WorkerMessagingProxy::~WorkerMessagingProxy()
@@ -162,8 +165,9 @@
 
 void WorkerMessagingProxy::postMessageToPageInspector(const String& message)
 {
-    RunLoop::main().dispatch([this, message = message.isolatedCopy()] {
-        m_inspectorProxy->sendMessageFromWorkerToFrontend(message);
+    RunLoop::main().dispatch([this, protectedThis = makeRef(*this), message = message.isolatedCopy()] {
+        if (!m_mayBeDestroyed)
+            m_inspectorProxy->sendMessageFromWorkerToFrontend(message);
     });
 }
 
@@ -233,8 +237,9 @@
 
     m_inspectorProxy->workerTerminated();
 
+    // This balances the original ref in construction.
     if (m_mayBeDestroyed)
-        delete this;
+        deref();
 }
 
 void WorkerMessagingProxy::terminateWorkerGlobalScope()

Modified: trunk/Source/WebCore/workers/WorkerMessagingProxy.h (215527 => 215528)

--- trunk/Source/WebCore/workers/WorkerMessagingProxy.h	2017-04-19 20:20:15 UTC (rev 215527)
+++ trunk/Source/WebCore/workers/WorkerMessagingProxy.h	2017-04-19 20:22:49 UTC (rev 215528)
@@ -28,6 +28,7 @@
 #include "WorkerGlobalScopeProxy.h"
 #include "WorkerLoaderProxy.h"
 #include "WorkerObjectProxy.h"
+#include <wtf/ThreadSafeRefCounted.h>
 
 namespace WebCore {
 
@@ -34,14 +35,13 @@
 class DedicatedWorkerThread;
 class WorkerInspectorProxy;
 
-class WorkerMessagingProxy final : public WorkerGlobalScopeProxy, public WorkerObjectProxy, public WorkerLoaderProxy {
+class WorkerMessagingProxy final : public ThreadSafeRefCounted<WorkerMessagingProxy>, public WorkerGlobalScopeProxy, public WorkerObjectProxy, public WorkerLoaderProxy {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     explicit WorkerMessagingProxy(Worker&);
+    virtual ~WorkerMessagingProxy();
 
 private:
-    virtual ~WorkerMessagingProxy();
-
     // Implementations of WorkerGlobalScopeProxy.
     // (Only use these functions in the worker object thread.)
     void startWorkerGlobalScope(const URL& scriptURL, const String& userAgent, const String& sourceCode, const ContentSecurityPolicyResponseHeaders&, bool shouldBypassMainWorldContentSecurityPolicy, MonotonicTime timeOrigin, JSC::RuntimeFlags) final;

_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes