Changeset 5522 in vbox for trunk/src/libs

Timestamp:

Oct 26, 2007 5:21:00 PM (17 years ago)

Author:

vboxsync

Message:

XPCOM: Fixed memory errors and leaks found by valgrind.

Location:

trunk/src/libs/xpcom18a4

Files:

• ipc/ipcd/client/src/ipcdclient.cpp (modified) (2 diffs)
• ipc/ipcd/extensions/dconnect/src/ipcDConnectService.cpp (modified) (8 diffs)
• xpcom/base/nsExceptionService.cpp (modified) (2 diffs)
• xpcom/glue/nsDebug.cpp (modified) (3 diffs)
• xpcom/glue/nsMemory.cpp (modified) (3 diffs)

trunk/src/libs/xpcom18a4/ipc/ipcd/client/src/ipcdclient.cpp

@@ -1198 +1198 @@
   td->pendingQ.Append(msg);
 
-  LOG(("placed message on pending queue for target %s and notifying all...\n", target.ToString()));
+#ifdef IPC_LOGGING
+  {
+    char *targetStr = target.ToString();
+    LOG(("placed message on pending queue for target %s and notifying all...\n", targetStr));
+    nsMemory::Free(targetStr);
+  }
+#endif
 
   // wake up anyone waiting on this queue
@@ -1277 +1283 @@
         nsAutoMonitor mon(gClientState->monitor);
         gClientState->targetMap.EnumerateRead(EnumerateTargetMapAndPlaceMsg, msg);
+
+        delete msg;
         
         return;

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

@@ -122 +122 @@
 // dconnect minor opcodes for INVOKE
 
+#pragma pack(1)
+
 struct DConnectOp
 {
@@ -186 +188 @@
   // (see ipcDConnectService::SerializeException)
 };
+
+#pragma pack()
 
 //-----------------------------------------------------------------------------
@@ -471 +475 @@
     case nsXPTType::T_IID:
       {
-        nsID *buf = (nsID *) malloc(sizeof(nsID));
+        nsID *buf = (nsID *) nsMemory::Alloc(sizeof(nsID));
         reader.GetBytes(buf, sizeof(nsID));
         v.val.p = v.ptr = buf;
@@ -489 +493 @@
         else
         {
-          char *buf = (char *) malloc(len + 1);
+          char *buf = (char *) nsMemory::Alloc(len + 1);
           reader.GetBytes(buf, len);
           buf[len] = char(0);
@@ -510 +514 @@
         else
         {
-          PRUnichar *buf = (PRUnichar *) malloc(len + 2);
+          PRUnichar *buf = (PRUnichar *) nsMemory::Alloc(len + 2);
           reader.GetBytes(buf, len);
           buf[len / 2] = PRUnichar(0);
@@ -621 +625 @@
     v.type = t;
     v.flags = nsXPTCVariant::PTR_IS_DATA;
+
+    // nsID, string and wstring types are not understood as dippers (see
+    // DIPPER_TYPE in xpidl.h) but they behave like dippers too. Therefore we
+    // need to treat them so manually.
+    switch (t.TagPart())
+    {
+      case nsXPTType::T_IID:
+      case nsXPTType::T_CHAR_STR:
+      case nsXPTType::T_WCHAR_STR:
+        // add VAL_IS_ALLOCD to cause FinishParam() to do cleanup
+        v.flags |= nsXPTCVariant::VAL_IS_ALLOCD;
+        break;
+      default:
+        break;
+    }
   }
 
@@ -633 +652 @@
 
   if (v.IsValAllocated())
-    free(v.val.p);
+      nsMemory::Free(v.val.p);
   else if (v.IsValInterface())
     ((nsISupports *) v.val.p)->Release();
@@ -1786 +1805 @@
 
     // check if this object is nsISupports itself
-    nsIID *iid = 0;
-    rv = mIInfo->GetInterfaceIID(&iid);
-    NS_ASSERTION(NS_SUCCEEDED(rv) && iid,
-                 "nsIInterfaceInfo::GetInterfaceIID failed");
-    if (NS_SUCCEEDED(rv) && iid &&
-        iid->Equals(NS_GET_IID(nsISupports)))
-    {
-      // nsISupports is queried on nsISupports, return ourselves
-      *aInstancePtr = this;
-      NS_ADDREF_THIS();
-      // cache ourselves weakly
-      mCachedISupports = this;
-
-      PR_Unlock(dConnect->StubQILock());
-      return NS_OK;
+    {
+      nsIID *iid = 0;
+      rv = mIInfo->GetInterfaceIID(&iid);
+      NS_ASSERTION(NS_SUCCEEDED(rv) && iid,
+                   "nsIInterfaceInfo::GetInterfaceIID failed");
+      if (NS_SUCCEEDED(rv) && iid &&
+          iid->Equals(NS_GET_IID(nsISupports)))
+      {
+        nsMemory::Free((void*)iid);
+
+        // nsISupports is queried on nsISupports, return ourselves
+        *aInstancePtr = this;
+        NS_ADDREF_THIS();
+        // cache ourselves weakly
+        mCachedISupports = this;
+
+        PR_Unlock(dConnect->StubQILock());
+        return NS_OK;
+      }
+      if (iid)
+        nsMemory::Free((void*)iid);
     }
 

trunk/src/libs/xpcom18a4/xpcom/base/nsExceptionService.cpp

@@ -84 +84 @@
 #ifdef NS_DEBUG
   inline nsrefcnt ReleaseQuiet() {
-      nsAutoOwningThread old = _mOwningThread;
-      _mOwningThread = nsAutoOwningThread();
-      nsrefcnt ref = Release();
+    // shut up NS_ASSERT_OWNINGTHREAD (see explaination below)
+    nsAutoOwningThread old = _mOwningThread;
+    _mOwningThread = nsAutoOwningThread();
+    nsrefcnt ref = Release();
+    NS_ASSERTION(ref == 0, "the object is still referenced by other threads while it shouldn't");
+    if (ref != 0)
       _mOwningThread = old;
-      return ref;
+    return ref;
   }
 #else  
@@ -103 +106 @@
 #endif
 
-// Note this object is single threaded - the service itself ensures
-// one per thread.
-// Exceptions are DropAllThreads (called on the thread shutting down xpcom)
-// and ThreadDestruct (called after xpcom destroyed the internal thread struct,
-// so that PR_GetCurrentThread() will create a new one from scratch which will
-// obviously not match the old one stored in the instance on creation). Since
-// these NS_CheckThreadSafe() assertions are quite annoying in the debug build,
-// we use a special ReleaseQuiet() mehtod in DoDropThread() to shut them up.
+// Note: the nsExceptionManager object is single threaded - the exception
+// service itself ensures one per thread. However, there are two methods that
+// may be called on foreign threads: DropAllThreads (called on the thread
+// shutting down xpcom) and ThreadDestruct (called after xpcom destroyed the
+// internal thread struct, so that PR_GetCurrentThread() will create a new one
+// from scratch which will obviously not match the old one stored in the
+// instance on creation). In both cases, there should be no other threads
+// holding objects (i.e. it's thread-safe to call them), but
+// NS_CheckThreadSafe() assertions will still happen and yell in the debug
+// build. Since it is quite annoying, we use a special ReleaseQuiet() mehtod
+// in DoDropThread() to shut them up.
 NS_IMPL_ISUPPORTS1(nsExceptionManager, nsIExceptionManager)
 

trunk/src/libs/xpcom18a4/xpcom/glue/nsDebug.cpp

@@ -44 +44 @@
 static nsIDebug* gDebugObject = nsnull;
 
+// Note: Although the machinery here is similar to nsMemory.cpp, we cannot use
+// NS_ASSERTION macros below because they end up in nsDebug::Assertion and
+// therefore may produce endless recursion.
+
 static NS_METHOD FreeDebugObject(void)
 {
@@ -55 +59 @@
 static nsIDebug* SetupDebugObject()
 {
-  NS_GetDebug(&gDebugObject);
-  if (gDebugObject)
-    NS_RegisterXPCOMExitRoutine(FreeDebugObject, 0);
+  if (!gDebugObject)
+  {
+    NS_GetDebug(&gDebugObject);
+    if (gDebugObject)
+      NS_RegisterXPCOMExitRoutine(FreeDebugObject, 0);
+  }
   return gDebugObject;
 }
@@ -64 +71 @@
 nsresult GlueStartupDebug() 
 {
-  NS_GetDebug(&gDebugObject);
-  if (!gDebugObject) 
-    return NS_ERROR_FAILURE;
+  if (!gDebugObject)
+  {
+    NS_GetDebug(&gDebugObject);
+    if (!gDebugObject) 
+      return NS_ERROR_FAILURE;
+  }
   return NS_OK;
 }

trunk/src/libs/xpcom18a4/xpcom/glue/nsMemory.cpp

@@ -44 +44 @@
 static NS_METHOD FreeGlobalMemory(void)
 {
+    NS_ASSERTION(gMemory, "must be not null after SetupGlobalMemory");
     NS_IF_RELEASE(gMemory);
     return NS_OK;
@@ -54 +55 @@
 SetupGlobalMemory()
 {
-    NS_ASSERTION(!gMemory, "bad call");
-    NS_GetMemoryManager(&gMemory);
-    NS_ASSERTION(gMemory, "can't get memory manager!");
-    NS_RegisterXPCOMExitRoutine(FreeGlobalMemory, 0);
+    NS_ASSERTION(!gMemory, "must be called once");
+    if (!gMemory)
+    {
+        NS_GetMemoryManager(&gMemory);
+        NS_ASSERTION(gMemory, "can't get memory manager!");
+        if (gMemory)
+            NS_RegisterXPCOMExitRoutine(FreeGlobalMemory, 0);
+    }
     return gMemory;
 }
@@ -64 +69 @@
 nsresult GlueStartupMemory() 
 {
-    NS_ASSERTION(!gMemory, "bad call");
-    NS_GetMemoryManager(&gMemory);
-    NS_ASSERTION(gMemory, "can't get memory manager!");
+    NS_ASSERTION(!gMemory, "must be called once");
+    if (!gMemory)
+    {
+        NS_GetMemoryManager(&gMemory);
+        NS_ASSERTION(gMemory, "can't get memory manager!");
+        if (!gMemory)
+            return NS_ERROR_FAILURE;
+    }
     return NS_OK;
 }