Bridge::destroy should wait for the completion of Peer::destroy

Bridge::initialize waits for the completion of Peer::initialize in order to avoid threading issues. Similarly, we should make Bridge::destroy wait for the completion of Peer::destroy to avoid threading issues. Specifically, the following threading issue can happen in oilpan builds:

(1) The worker thread calls Bridge::destroy.
(2) The worker thread finishes and calls a final GC to clean up the thread heap before Peer::destroy is triggered.

Then the final GC hits the empty ASSERT (which checks if the thread heap of the worker thread is empty) because the main thread still holds a reference to the SyncHelper, which is allocated on the thread heap of the worker thread. In order to make sure that the worker thread runs the final GC after the main thread clears the references to all objects allocated in the thread heap of the worker thread, we need to make the Bridge::destroy wait for the main thread to complete the Peer::destroy.

This issue doesn't happen in non-oilpan builds, so this CL adds the synchronization to oilpan builds only. (In any event, we cannot add the synchronization to non-oilpan builds, because Peer::destroy destructs the SyncHelper object and thus there is no way for the main thread to tell the worker thread that the main thread completes the Peer::destroy.)

Review URL: https://codereview.chromium.org/339663004

git-svn-id: svn://svn.chromium.org/blink/trunk@176751 bbb929c8-8fbe-4397-9dbb-9b2b20218538

1 files changed, 9 insertions, 0 deletions

diff --git a/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp b/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp
index c172874c00fa..7f82ad89ac2a 100644
--- a/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp
+++ b/Source/modules/websockets/WorkerThreadableWebSocketChannel.cpp
@@ -254,6 +254,7 @@ void WorkerThreadableWebSocketChannel::Peer::destroy()
 
 #if ENABLE(OILPAN)
     m_keepAlive = nullptr;
+    m_syncHelper->signalWorkerThread();
 #else
     delete this;
 #endif
@@ -567,7 +568,15 @@ bool WorkerThreadableWebSocketChannel::Bridge::waitForMethodCompletion(PassOwnPt
 void WorkerThreadableWebSocketChannel::Bridge::terminatePeer()
 {
     ASSERT(!hasTerminatedPeer());
+#if ENABLE(OILPAN)
+    // The worker thread has to wait for the main thread to complete Peer::destroy,
+    // because the worker thread has to make sure that the main thread does not have any
+    // references to on-heap objects allocated in the thread heap of the worker thread
+    // before the worker thread shuts down.
+    waitForMethodCompletion(CallClosureTask::create(bind(&Peer::destroy, m_peer)));
+#else
     m_loaderProxy.postTaskToLoader(CallClosureTask::create(bind(&Peer::destroy, m_peer)));
+#endif
 
     // Peer::destroy() deletes m_peer and then m_syncHelper will be released.
     // We must not touch m_syncHelper any more.