Manually refcount ReceivedSyncMsgQueue so that we force it to be destructed on

the listener thread.  I've fixed the renderer crashes which were happening because
SyncChannel was going away before the listener thread.


git-svn-id: svn://svn.chromium.org/chrome/trunk/src@762 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 20 insertions, 10 deletions

diff --git a/chrome/common/ipc_sync_channel.cc b/chrome/common/ipc_sync_channel.cc
index e01759a..e29d330 100644
--- a/chrome/common/ipc_sync_channel.cc
+++ b/chrome/common/ipc_sync_channel.cc
@@ -69,7 +69,10 @@ class SyncChannel::ReceivedSyncMsgQueue :
   }
 
   ~ReceivedSyncMsgQueue() {
+    DCHECK(ThreadLocalStorage::Get(g_tls_index));
+    DCHECK(MessageLoop::current() == listener_message_loop_);
     CloseHandle(blocking_event_);
+    ThreadLocalStorage::Set(g_tls_index, NULL);
   }
 
   // Called on IPC thread when a synchronous message or reply arrives.
@@ -182,6 +185,7 @@ class SyncChannel::ReceivedSyncMsgQueue :
   }
 
   HANDLE blocking_event() { return blocking_event_; }
+  MessageLoop* listener_message_loop() { return listener_message_loop_; }
 
  private:
   // Called on the ipc thread to check if we can unblock any current Send()
@@ -247,24 +251,24 @@ SyncChannel::SyncContext::SyncContext(
   received_sync_msgs_ = static_cast<ReceivedSyncMsgQueue*>(
       ThreadLocalStorage::Get(g_tls_index));
 
-  if (!received_sync_msgs_.get()) {
+  if (!received_sync_msgs_) {
     // Stash a pointer to the listener thread's ReceivedSyncMsgQueue, as we
     // need to be able to access it in the IPC thread.
     received_sync_msgs_ = new ReceivedSyncMsgQueue();
-    // TODO(jcampan): http:///b/1319842 we are adding an extra-ref to keep this
-    // object around for the duration of the process.  We used to remove it from
-    // the TLS when it was destroyed, but that was causing problems as we could
-    // be destroyed in a different thread then the thread we had been created
-    // on.  This needs to be revisited at some point.
-    received_sync_msgs_->AddRef();
-
-    ThreadLocalStorage::Set(g_tls_index, received_sync_msgs_.get());
+    ThreadLocalStorage::Set(g_tls_index, received_sync_msgs_);
   }
+
+  // Addref manually so that we can ensure destruction on the listener thread
+  // (so that the TLS object is NULLd).
+  received_sync_msgs_->AddRef();
 }
 
 SyncChannel::SyncContext::~SyncContext() {
   while (!deserializers_.empty())
     PopDeserializer(true);
+
+  received_sync_msgs_->listener_message_loop()->ReleaseSoon(
+      FROM_HERE, received_sync_msgs_);
 }
 
 // Adds information about an outgoing sync message to the context so that
diff --git a/chrome/common/ipc_sync_channel.h b/chrome/common/ipc_sync_channel.h
index bd473d4..fa3566e 100644
--- a/chrome/common/ipc_sync_channel.h
+++ b/chrome/common/ipc_sync_channel.h
@@ -128,7 +128,9 @@ class SyncChannel : public ChannelProxy {
     PendingSyncMessageQueue deserializers_;
     Lock deserializers_lock_;
 
-    scoped_refptr<ReceivedSyncMsgQueue> received_sync_msgs_;
+    // This can't be a scoped_refptr because it needs to be released on the
+    // listener thread.
+    ReceivedSyncMsgQueue* received_sync_msgs_;
 
     bool channel_closed_;
     bool reply_deserialize_result_;
diff --git a/chrome/renderer/render_thread.cc b/chrome/renderer/render_thread.cc
index b61e1eb..ddc8ebd 100644
--- a/chrome/renderer/render_thread.cc
+++ b/chrome/renderer/render_thread.cc
@@ -135,6 +135,10 @@ void RenderThread::Init() {
 void RenderThread::CleanUp() {
   DCHECK(current() == this);
 
+  // Need to destruct the SyncChannel to the browser before we go away because
+  // it caches a pointer to this thread.
+  channel_.reset();
+
   // Clean up plugin channels before this thread goes away.
   PluginChannelBase::CleanupChannels();