Bug 556194 - Use-after-free of PluginInstanceChild::mAsyncCallMutex - don't clear the pending async calls until after NPP_Destroy is called, because it is a synchronization point. r=cjones a=blocker
authorBenjamin Smedberg <benjamin@smedbergs.us>
Tue, 07 Dec 2010 10:05:56 -0500
changeset 58777 f37d738ce296e958910f3b68777b4cf2d52ae75f
parent 58776 7de7681aa6d348594e95321320a8177e36e64be6
child 58778 ded653253911202b8e1723927bbe8f83160e42a4
push id1
push usershaver@mozilla.com
push dateTue, 04 Jan 2011 17:58:04 +0000
reviewerscjones, blocker
bugs556194
milestone2.0b8pre
Bug 556194 - Use-after-free of PluginInstanceChild::mAsyncCallMutex - don't clear the pending async calls until after NPP_Destroy is called, because it is a synchronization point. r=cjones a=blocker
dom/plugins/PluginInstanceChild.cpp
--- a/dom/plugins/PluginInstanceChild.cpp
+++ b/dom/plugins/PluginInstanceChild.cpp
@@ -2963,33 +2963,29 @@ PluginInstanceChild::AnswerNPP_Destroy(N
         if (static_cast<BrowserStreamChild*>(streams[i])->InstanceDying())
             ++i;
         else
             streams.RemoveElementAt(i);
     }
     for (PRUint32 i = 0; i < streams.Length(); ++i)
         static_cast<BrowserStreamChild*>(streams[i])->FinishDelivery();
 
-    {
-        MutexAutoLock lock(mAsyncCallMutex);
-        for (PRUint32 i = 0; i < mPendingAsyncCalls.Length(); ++i)
-            mPendingAsyncCalls[i]->Cancel();
-        mPendingAsyncCalls.TruncateLength(0);
-    }
-
     mTimers.Clear();
     if (mCurrentInvalidateTask) {
         mCurrentInvalidateTask->Cancel();
         mCurrentInvalidateTask = nsnull;
     }
     if (mCurrentAsyncSetWindowTask) {
         mCurrentAsyncSetWindowTask->Cancel();
         mCurrentAsyncSetWindowTask = nsnull;
     }
 
+    // NPP_Destroy() should be a synchronization point for plugin threads
+    // calling NPN_AsyncCall: after this function returns, they are no longer
+    // allowed to make async calls on this instance.
     PluginModuleChild::current()->NPP_Destroy(this);
     mData.ndata = 0;
 
     mDeletingHash = new nsTHashtable<DeletingObjectEntry>;
     mDeletingHash->Init();
     PluginModuleChild::current()->FindNPObjectsForInstance(this);
 
     mDeletingHash->EnumerateEntries(InvalidateObject, NULL);
@@ -3001,10 +2997,17 @@ PluginInstanceChild::AnswerNPP_Destroy(N
     mCachedElementActor = nsnull;
 
 #if defined(OS_WIN)
     SharedSurfaceRelease();
     DestroyWinlessPopupSurrogate();
     UnhookWinlessFlashThrottle();
 #endif
 
+    // Pending async calls are discarded, not delivered. This matches the
+    // in-process behavior.
+    for (PRUint32 i = 0; i < mPendingAsyncCalls.Length(); ++i)
+        mPendingAsyncCalls[i]->Cancel();
+
+    mPendingAsyncCalls.Clear();
+
     return true;
 }