Changeset 103615 in vbox for trunk/src/VBox/GuestHost/SharedClipboard

Timestamp:

Feb 29, 2024 6:36:37 PM (11 months ago)

Author:

vboxsync

Message:

Shared Clipboard/Transfers: Made code a bit easier to follow / read, removed some unnecessary locking (cleanup). bugref:9437

Location:

trunk/src/VBox/GuestHost/SharedClipboard

Files:

• ClipboardDataObjectImpl-win.cpp (modified) (16 diffs)
• clipboard-transfers.cpp (modified) (2 diffs)
• clipboard-win.cpp (modified) (2 diffs)

trunk/src/VBox/GuestHost/SharedClipboard/ClipboardDataObjectImpl-win.cpp

@@ -219 +219 @@
 
         lock();
-
-        /* Make sure to release the transfer. */
-        setTransferLocked(NULL);
-    }
+    }
+
+    /* Make sure to release the transfer in any state. */
+    setTransferLocked(NULL);
 
     unlock();
@@ -280 +280 @@
         m_pStgMedium = NULL;
     }
-
-    if (m_pTransfer)
-        ShClTransferRelease(m_pTransfer);
 
     FsObjEntryList::const_iterator itRoot = m_lstEntries.cbegin();
@@ -566 +563 @@
             if (RT_SUCCESS(rc))
             {
-                pThis->lock();
-
                 AssertReleaseMsg(pThis->m_lstEntries.size(),
                                  ("Shared Clipboard: No transfer root entries found -- should not happen, please file a bug report\n"));
@@ -578 +573 @@
                         break;
 
-                    pThis->unlock();
-
                     /* Transferring stuff can take a while, so don't use any timeout here. */
                     rc = RTSemEventWait(pThis->m_EventStatusChanged, RT_INDEFINITE_WAIT);
-
-                    pThis->lock();
-
                     if (RT_FAILURE(rc))
                         break;
@@ -620 +610 @@
                             break;
                     }
-
-                    pThis->unlock();
 
                     if (pThis->m_Callbacks.pfnTransferEnd)
@@ -630 +618 @@
                     }
 
-                    pThis->lock();
-
                     break;
-                }
-
-                pThis->unlock();
+                } /* for */
             }
         }
@@ -806 +790 @@
     int rc = VINF_SUCCESS;
 
-    /* Pre-check -- see if the data object still is alive. */
-    if (m_enmStatus == Uninitialized)
-        rc = VERR_OBJECT_DESTROYED;
-
     if (    RT_SUCCESS(rc)
         && (   pFormatEtc->cfFormat == m_cfFileDescriptorA
@@ -841 +821 @@
                 lock();
 
+                LogFunc(("Wait resulted in %Rrc and status %#x\n", rc, m_enmStatus));
+
                 if (RT_FAILURE(rc))
                 {
@@ -864 +846 @@
                 const bool fUnicode = pFormatEtc->cfFormat == m_cfFileDescriptorW;
 
+                /* Leave lock while waiting. */
+                unlock();
+
                 SHCLTRANSFERSTATUS const enmTransferStatus = ShClTransferGetStatus(m_pTransfer);
-                RT_NOREF(enmTransferStatus);
 
                 LogFlowFunc(("FormatIndex_FileDescriptor%s, enmTransferStatus=%s\n",
@@ -871 +855 @@
 
                 /* The caller can call GetData() several times, so make sure we don't do the same transfer multiple times. */
-                if (ShClTransferGetStatus(m_pTransfer) != SHCLTRANSFERSTATUS_STARTED)
+                if (enmTransferStatus != SHCLTRANSFERSTATUS_STARTED)
                 {
                     /* Start the transfer + run it asynchronously in a separate thread. */
@@ -880 +864 @@
                         if (RT_SUCCESS(rc))
                         {
-                            /* Leave lock while waiting. */
-                            unlock();
-
                             /* Don't block for too long here, as this also will screw other apps running on the OS. */
                             LogRel2(("Shared Clipboard: Waiting for IDataObject listing to arrive ...\n"));
@@ -893 +874 @@
                                 || m_enmStatus != Running) /* Still in running state? */
                             {
-                                rc = VERR_OBJECT_DESTROYED;
+                                rc = VERR_SHCLPB_NO_DATA;
                                 break;
                             }
+
+                            unlock();
                         }
                     }
                 }
+
+                lock();
 
                 if (RT_SUCCESS(rc))
@@ -1147 +1132 @@
  * @returns VBox status code.
  * @param   pTransfer           Transfer to assign.
- *                              Must be in INITIALIZED state.
  *                              When set to NULL, the transfer will be released from the object.
  */
@@ -1154 +1138 @@
     AssertReturn(RTCritSectIsOwned(&m_CritSect), VERR_WRONG_ORDER);
 
-    LogFlowFunc(("pTransfer=%p\n", pTransfer));
+    LogFunc(("pTransfer=%p\n", pTransfer));
 
     int rc = VINF_SUCCESS;
@@ -1164 +1148 @@
         if (m_enmStatus == Initialized)
         {
-            SHCLTRANSFERSTATUS const enmSts = ShClTransferGetStatus(pTransfer);
-            AssertMsgStmt(enmSts == SHCLTRANSFERSTATUS_INITIALIZED, /* Transfer must not be started yet. */
-                          ("Transfer has wrong status (%#x)\n", enmSts), rc = VERR_WRONG_ORDER);
-            if (RT_SUCCESS(rc))
-            {
-                m_pTransfer = pTransfer;
-
-                SharedClipboardWinTransferCtx *pWinURITransferCtx = (SharedClipboardWinTransferCtx *)pTransfer->pvUser;
-                AssertPtr(pWinURITransferCtx);
-
-                pWinURITransferCtx->pDataObj = this; /* Save a backref to this object. */
-
-                ShClTransferAcquire(pTransfer);
-            }
+            m_pTransfer = pTransfer;
+
+            SharedClipboardWinTransferCtx *pWinURITransferCtx = (SharedClipboardWinTransferCtx *)pTransfer->pvUser;
+            AssertPtr(pWinURITransferCtx);
+
+            pWinURITransferCtx->pDataObj = this; /* Save a backref to this object. */
+
+            ShClTransferAcquire(pTransfer);
         }
         else
@@ -1208 +1186 @@
  * @returns VBox status code.
  * @param   pTransfer           Transfer to assign.
- *                              Must be in INITIALIZED state.
  *                              When set to NULL, the transfer will be released from the object.
  */

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

@@ -1234 +1234 @@
 
     int rc = VINF_SUCCESS;
+
     if (pTransfer->Callbacks.pfnOnInitialize)
         rc = pTransfer->Callbacks.pfnOnInitialize(&pTransfer->CallbackCtx);
-
     if (RT_SUCCESS(rc))
     {
@@ -1244 +1244 @@
             AssertMsgStmt(ShClTransferRootsCount(pTransfer), ("Transfer has no root entries set\n"), rc = VERR_WRONG_ORDER);
 
-        rc = shClTransferSetStatus(pTransfer, SHCLTRANSFERSTATUS_INITIALIZED);
-
-        if (   RT_SUCCESS(rc)
-            && pTransfer->Callbacks.pfnOnInitialized)
-            pTransfer->Callbacks.pfnOnInitialized(&pTransfer->CallbackCtx);
+        if (RT_SUCCESS(rc))
+            rc = shClTransferSetStatus(pTransfer, SHCLTRANSFERSTATUS_INITIALIZED);
     }
 
     shClTransferUnlock(pTransfer);
 
+    /* Note: Callback will be called after we unlocked the transfer, as the caller might access the transfer right away. */
+    if (   RT_SUCCESS(rc)
+        && pTransfer->Callbacks.pfnOnInitialized)
+        pTransfer->Callbacks.pfnOnInitialized(&pTransfer->CallbackCtx);
+
     if (RT_FAILURE(rc))
-        LogRel2(("Shared Clipboard: Initialziation of transfer failed with %Rrc\n", rc));
+        LogRel(("Shared Clipboard: Initialziation of transfer failed with %Rrc\n", rc));
 
     LogFlowFuncLeaveRC(rc);

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

@@ -1192 +1192 @@
 
 /**
- * Hands off a transfer to the current data object in-flight.
+ * Initializes a Windows transfer for a given data object.
  *
  * @returns VBox status code.
  * @param   pWinCtx             Windows context to use.
- * @param   pTransfer           Transfer to hand off to the data object.
- *
- * @note    The data object will be put into running state on success and handed over to Windows.
- *          Our data object pointer will be invalid afterwards.
- */
-int SharedClipboardWinTransferHandOffToDataObject(PSHCLWINCTX pWinCtx, PSHCLTRANSFER pTransfer)
+ * @param   pTransfer           Transfer to initialize for the data object.
+ * @param   pObj                Data object to initialize transfer for.
+ */
+static int sharedClipboardWinTransferInitializeInternal(PSHCLWINCTX pWinCtx, PSHCLTRANSFER pTransfer,
+                                                        SharedClipboardWinDataObject *pObj)
+{
+    RT_NOREF(pWinCtx);
+
+    return pObj->SetTransfer(pTransfer);
+}
+
+/**
+ * Initializes a Windows transfer for the current data object in-flight.
+ *
+ * @returns VBox status code.
+ * @retval  VERR_WRONG_ORDER if no current in-flight data object is available.
+ * @param   pWinCtx             Windows context to use.
+ * @param   pTransfer           Transfer to initialize for the data object.
+ */
+int SharedClipboardWinTransferInitialize(PSHCLWINCTX pWinCtx, PSHCLTRANSFER pTransfer)
 {
     int rc = RTCritSectEnter(&pWinCtx->CritSect);
@@ -1209 +1223 @@
         if (pObj)
         {
-            rc = pObj->SetTransfer(pTransfer);
+            rc = sharedClipboardWinTransferInitializeInternal(pWinCtx, pTransfer, pObj);
+        }
+        else /* No current in-flight data object. */
+            rc = VERR_WRONG_ORDER;
+
+        int rc2 = RTCritSectLeave(&pWinCtx->CritSect);
+        AssertRC(rc2);
+    }
+
+    return rc;
+}
+
+/**
+ * Starts a Windows transfer for a given data object.
+ *
+ * @returns VBox status code.
+ * @param   pWinCtx             Windows context to use.
+ * @param   pObj                Data object to start transfer for.
+ */
+static int sharedClipboardWinTransferStartInternal(PSHCLWINCTX pWinCtx, SharedClipboardWinDataObject *pObj)
+{
+    RT_NOREF(pWinCtx);
+
+    return pObj->SetStatus(SharedClipboardWinDataObject::Running);
+}
+
+/**
+ * Starts a Windows transfer for the current data object in-flight.
+ *
+ * This hands off the data object in-flight to Windows on success.
+ *
+ * @returns VBox status code.
+ * @retval  VERR_WRONG_ORDER if no current in-flight data object is available.
+ * @param   pWinCtx             Windows context to use.
+ * @param   pTransfer           Transfer to initialize for the data object.
+ */
+int SharedClipboardWinTransferStart(PSHCLWINCTX pWinCtx, PSHCLTRANSFER pTransfer)
+{
+    RT_NOREF(pTransfer);
+
+    int rc = RTCritSectEnter(&pWinCtx->CritSect);
+    if (RT_SUCCESS(rc))
+    {
+        SharedClipboardWinDataObject *pObj = pWinCtx->pDataObjInFlight;
+        if (pObj)
+        {
+            rc = sharedClipboardWinTransferStartInternal(pWinCtx, pObj);
             if (RT_SUCCESS(rc))
-                rc = pObj->SetStatus(SharedClipboardWinDataObject::Running);
-
-            pWinCtx->pDataObjInFlight = NULL; /* Hand off to Windows. */
-        }
-        else
-        {
-            AssertMsgFailed(("No data object in flight (yet)!\n"));
+                pWinCtx->pDataObjInFlight = NULL; /* Hand off to Windows on success. */
+        }
+        else /* No current in-flight data object. */
             rc = VERR_WRONG_ORDER;
-        }
 
         int rc2 = RTCritSectLeave(&pWinCtx->CritSect);