Changeset 100641 in vbox for trunk

Timestamp:

Jul 19, 2023 8:33:05 AM (19 months ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

158478

Message:

Shared Clipboard: Easier to follow event destruction handling: Destroy + de-allocate an event if its refcount reaches 0. bugref:9437

Location:

trunk/src/VBox

Files:

• GuestHost/SharedClipboard/clipboard-common.cpp (modified) (6 diffs)
• HostServices/SharedClipboard/testcase/tstClipboardTransfers.cpp (modified) (1 diff)

trunk/src/VBox/GuestHost/SharedClipboard/clipboard-common.cpp

@@ -49 +49 @@
 *********************************************************************************************************************************/
 static void shClEventSourceResetInternal(PSHCLEVENTSOURCE pSource);
+static int shClEventSourceUnregisterEvent(PSHCLEVENTSOURCE pSource, PSHCLEVENT pEvent);
 
 static void shClEventDestroy(PSHCLEVENT pEvent);
@@ -204 +205 @@
     RTListForEachSafe(&pSource->lstEvents, pEvIt, pEvItNext, SHCLEVENT, Node)
     {
-        RTListNodeRemove(&pEvIt->Node);
+        bool const fDealloc = ASMAtomicReadU32(&pEvIt->cRefs) == 0; /* Still any references left? Skip de-allocation. */
+        if (!fDealloc)
+            Log3Func(("Event %RU32 has %RU32 references left, skipping de-allocation\n", pEvIt->idEvent, pEvIt->cRefs));
 
         shClEventDestroy(pEvIt);
 
-        RTMemFree(pEvIt);
-        pEvIt = NULL;
+        int rc2 = shClEventSourceUnregisterEvent(pSource, pEvIt);
+        AssertRC(rc2);
+
+        if (fDealloc)
+        {
+            RTMemFree(pEvIt);
+            pEvIt = NULL;
+        }
     }
 }
@@ -304 +313 @@
         return;
 
-    AssertMsgReturnVoid(pEvent->cRefs == 0, ("Event %RU32 still has %RU32 references\n",
-                                             pEvent->idEvent, pEvent->cRefs));
-
     LogFlowFunc(("Event %RU32\n", pEvent->idEvent));
 
@@ -326 +332 @@
  * @returns VBox status code.
  * @param   pSource             Event source to unregister event for.
- * @param   pEvent              Event to unregister. On success the pointer will be invalid.
- */
-static int shClEventSourceUnregisterEventInternal(PSHCLEVENTSOURCE pSource, PSHCLEVENT pEvent)
-{
+ * @param   pEvent              Event to unregister.
+ */
+static int shClEventSourceUnregisterEvent(PSHCLEVENTSOURCE pSource, PSHCLEVENT pEvent)
+{
+    RT_NOREF(pSource);
+
     LogFlowFunc(("idEvent=%RU32, cRefs=%RU32\n", pEvent->idEvent, pEvent->cRefs));
 
-    AssertReturn(pEvent->cRefs == 0, VERR_WRONG_ORDER);
-
-    int rc = RTCritSectEnter(&pSource->CritSect);
-    if (RT_SUCCESS(rc))
-    {
-        RTListNodeRemove(&pEvent->Node);
-
-        shClEventDestroy(pEvent);
-
-        rc = RTCritSectLeave(&pSource->CritSect);
-        if (RT_SUCCESS(rc))
-        {
-            RTMemFree(pEvent);
-            pEvent = NULL;
-        }
-    }
-
-    return rc;
+    RTListNodeRemove(&pEvent->Node);
+    pEvent->pParent = NULL;
+
+    return VINF_SUCCESS;
 }
 
@@ -522 +516 @@
 
 /**
- * Releases event by decreasing its reference count. Will be destroys once the reference count reaches 0.
+ * Releases event by decreasing its reference count. Will be destroyed once the reference count reaches 0.
  *
  * @returns New reference count, or UINT32_MAX if failed.
@@ -539 +533 @@
     if (cRefs == 0)
     {
-        AssertPtr(pEvent->pParent);
-        int rc2 = shClEventSourceUnregisterEventInternal(pEvent->pParent, pEvent);
-        AssertRC(rc2);
-
-        return RT_SUCCESS(rc2) ? 0 : UINT32_MAX;
+        int rc;
+        PSHCLEVENTSOURCE pParent = pEvent->pParent;
+        if (   pParent
+            && RTCritSectIsInitialized(&pParent->CritSect))
+        {
+            rc = RTCritSectEnter(&pParent->CritSect);
+            if (RT_SUCCESS(rc))
+            {
+                rc = shClEventSourceUnregisterEvent(pParent, pEvent);
+
+                int rc2 = RTCritSectLeave(&pParent->CritSect);
+                if (RT_SUCCESS(rc))
+                    rc = rc2;
+            }
+        }
+        else
+            rc = VINF_SUCCESS;
+
+        if (RT_SUCCESS(rc))
+        {
+            shClEventDestroy(pEvent);
+
+            RTMemFree(pEvent);
+            pEvent = NULL;
+        }
+
+        return RT_SUCCESS(rc) ? 0 : UINT32_MAX;
     }
 

trunk/src/VBox/HostServices/SharedClipboard/testcase/tstClipboardTransfers.cpp

@@ -309 +309 @@
     RTTESTI_CHECK_RC_OK(ShClEventSourceGenerateAndRegisterEvent(&Source, &pEvent));
     ShClEventSourceReset(&Source);
-    RTTESTI_CHECK(ShClEventSourceGetLast(&Source) != NULL); /* Event still in, as it holds a reference. */
+    RTTESTI_CHECK(ShClEventSourceGetLast(&Source) == NULL); /* Event still valid, but removed from the source. */
     RTTESTI_CHECK(ShClEventRelease(pEvent) == 0);
     RTTESTI_CHECK(ShClEventRelease(pEvent) == UINT32_MAX); /* Ref count already was 0, so returns UINT32_MAX. */