Changeset 3036 in vbox for trunk/src

Timestamp:

Jun 4, 2007 7:19:11 PM (18 years ago)

Author:

vboxsync

Message:

XPCOM/IPC/DConnect: Fixed: Count the number of IPC references for object wrappers separately from the total number of references to avoid deletion of wrappers still being in use by the DConnect code upon client death; Fixed: Unbalanced AddRef() when processing the RELEASE client message.

File:

• trunk/src/libs/xpcom18a4/ipc/ipcd/extensions/dconnect/src/ipcDConnectService.cpp (modified) (8 diffs)

trunk/src/libs/xpcom18a4/ipc/ipcd/extensions/dconnect/src/ipcDConnectService.cpp

@@ -247 +247 @@
     return count;
   }
+
+  // this gets called after calling AddRef() on an instance passed to the
+  // client over IPC in order to have a count of IPC client-related references
+  // separately from the overall reference count
+  NS_IMETHODIMP_(nsrefcnt) AddRefIPC(void)
+  {
+    NS_PRECONDITION(PRInt32(mRefCntIPC) >= 0, "illegal refcnt");
+    nsrefcnt count;
+    count = PR_AtomicIncrement((PRInt32*)&mRefCntIPC);
+    return count;
+  }
+
+  // this gets called before calling Release() when DCON_OP_RELEASE is
+  // received from the IPC client
+  NS_IMETHODIMP_(nsrefcnt) ReleaseIPC(void)
+  {
+    NS_PRECONDITION(0 != mRefCntIPC, "dup release");
+    nsrefcnt count;
+    count = PR_AtomicDecrement((PRInt32 *)&mRefCntIPC);
+    return count;
+  }
   
 private:
   nsAutoRefCnt               mRefCnt;
+  nsAutoRefCnt               mRefCntIPC;
   PRUint32                   mPeer;  // peer process "owning" this instance
   nsCOMPtr<nsIInterfaceInfo> mIInfo;
@@ -259 +281 @@
 {
   for (PRInt32 i=0; i<wrappers.Count(); ++i)
+  {
+    ((DConnectInstance *) wrappers[i])->ReleaseIPC();
     ((DConnectInstance *) wrappers[i])->Release();
+  }
 }
 
@@ -1106 +1131 @@
 
       // wrapper remains referenced when passing it to the client
-      // (will be released upon DCON_OP_RELEASE)
+      // (will be released upon DCON_OP_RELEASE). increase the
+      // second, IPC-only, reference counter
+      wrapper->AddRefIPC();
       
       // send address of the instance wrapper, and set the low bit
@@ -1373 +1400 @@
 
         // wrapper remains referenced when passing it to the client
-        // (will be released upon DCON_OP_RELEASE)
-      
+        // (will be released upon DCON_OP_RELEASE). increase the
+        // second, IPC-only, reference counter
+        wrapper->AddRefIPC();
+
         // send address of the instance wrapper, and set the low bit
         // to indicate that this is an instance wrapper.
@@ -2674 +2703 @@
   if (args && args->clientID == aData->Peer())
   {
-    // ignore the reference counter: the client is officially dead
-    args->that->DeleteInstance(aData, PR_TRUE /* locked */);
+    // add a fake reference to hold the wrapper alive
+    nsrefcnt count = aData->AddRef();
+
+    // release all IPC references for this wrapper, the client is now
+    // officially dead (and thus cannot call AddRefIPC in the middle)
+    nsrefcnt countIPC = aData->AddRefIPC();
+    countIPC = aData->ReleaseIPC();
+
+    LOG(("ipcDConnectService::PruneInstanceMapForPeer: "
+         "instance=%p: %d IPC refs to release (total refcnt=%d)\n",
+         aData, countIPC, count));
+
+    while (countIPC)
+    {
+      countIPC = aData->ReleaseIPC();
+      aData->Release();
+    }
+
     // collect the instance for future destruction
     if (!args->wrappers.AppendElement(aData))
@@ -2717 +2762 @@
       }
 
-      // destruct all instances outside the lock because it will release
-      // the real objects which may need to make asynchronous use our service
+      LOG(("ipcDConnectService::OnClientStateChange: "
+           "%d lost instances (should be 0 unless the peer has "
+           "crashed)\n", wrappers.Count()));
+
+      // release all fake references we've added in PruneInstanceMapForPeer().
+      // this may call wrapper destructors so it's important to do that
+      // outside the lock because destructors will release the real
+      // objects which may need to make asynchronous use our service
       for (PRInt32 i = 0; i < wrappers.Count(); ++i)
-        delete ((DConnectInstance *) wrappers[i]);
+        ((DConnectInstance *) wrappers[i])->Release();
     }
   }
@@ -2876 +2927 @@
 
       // wrapper remains referenced when passing it to the client
-      // (will be released upon DCON_OP_RELEASE)
+      // (will be released upon DCON_OP_RELEASE). increase the
+      // second, IPC-only, reference counter
+      wrapper->AddRefIPC();
     }
   }
@@ -2947 +3000 @@
   if (mInstanceSet.Contains(wrapper))
   {
+    // add a fake reference to hold the wrapper alive
     nsrefcnt count = wrapper->AddRef();
+
+    // release references
+    nsrefcnt countIPC = wrapper->ReleaseIPC();
     count = wrapper->Release();
+
+    NS_ASSERTION(count > 0, "unbalanced AddRef()/Release()");
+
     if (count == 1)
     {
+      NS_ASSERTION(countIPC == 0, "unbalanced AddRefIPC()/ReleaseIPC()");
+
       // we are the last one who holds a (fake) reference, remove the
       // instace from instance maps while still under the lock
       DeleteInstance(wrapper, PR_TRUE /* locked */);
+
       // leave the lock before calling the destructor because it will release
       // the real object which may need to make asynchronous use our service
       lock.unlock();
       delete wrapper;
+    }
+    else
+    {
+      // release the fake reference
+      wrapper->Release();
     }
   }