Changeset 100461 in vbox for trunk/src/VBox

Timestamp:

Jul 10, 2023 2:23:36 PM (17 months ago)

Author:

vboxsync

Message:

Shared Clipboard: More fine-grained control for Windows data object uninit vs. destruction. bugref:9437

File:

• trunk/src/VBox/GuestHost/SharedClipboard/ClipboardDataObjectImpl-win.cpp (modified) (17 diffs)

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

@@ -62 +62 @@
     , m_pStream(NULL)
     , m_uObjIdx(0)
-    , m_fThreadRunning(false)
     , m_EventListComplete(NIL_RTSEMEVENT)
     , m_EventStatusChanged(NIL_RTSEMEVENT)
@@ -180 +179 @@
 
 /**
- * Destroys a data object instance.
- */
-void SharedClipboardWinDataObject::Destroy(void)
+ * Uninitialized a data object instance, internal version.
+ */
+void SharedClipboardWinDataObject::uninitInternal(void)
 {
     LogFlowFuncEnter();
 
-    AssertReturnVoid(m_lRefCount == 0);
+    /* Let waiters know. */
+    setStatusLocked(Uninitialized, VINF_SUCCESS);
 
     /* Make sure to release the transfer. */
     setTransferLocked(NULL);
+}
+
+/**
+ * Uninitialized a data object instance.
+ */
+void SharedClipboardWinDataObject::Uninit(void)
+{
+    LogFlowFuncEnter();
+
+    lock();
+
+    uninitInternal();
+
+    unlock();
+}
+
+/**
+ * Destroys a data object instance.
+ */
+void SharedClipboardWinDataObject::Destroy(void)
+{
+    LogFlowFuncEnter();
+
+    if (m_enmStatus == Uninitialized) /* Crit sect not available anymore. */
+        return;
+
+    lock();
+
+    uninitInternal();
+
+    unlock();
 
     int rc = RTCritSectDelete(&m_CritSect);
@@ -236 +267 @@
     }
     m_lstEntries.clear();
-
-    m_enmStatus = Uninitialized;
-    m_fThreadRunning = false;
-    m_pTransfer = NULL;
 }
 
@@ -390 +417 @@
                                         objEntry.objInfo = *pFsObjInfo;
 
+                                        lock();
                                         m_lstEntries.push_back(objEntry); /** @todo Can this throw? */
+                                        unlock();
                                     }
                                     else /* Not fatal, just skip. */
@@ -477 +506 @@
                 objEntry.objInfo = *pFsObjInfo;
 
+                pThis->lock();
                 pThis->m_lstEntries.push_back(objEntry); /** @todo Can this throw? */
+                pThis->unlock();
 
                 rc = pThis->readDir(pTransfer, pRootEntry->pszName);
@@ -488 +519 @@
                 objEntry.objInfo = *pFsObjInfo;
 
+                pThis->lock();
                 pThis->m_lstEntries.push_back(objEntry); /** @todo Can this throw? */
+                pThis->unlock();
             }
             else
@@ -516 +549 @@
              * This in turn then will create IStream instances (by the OS) for each file system object to handle.
              */
-            int rc2 = RTSemEventSignal(pThis->m_EventListComplete);
-            AssertRC(rc2);
-
-            if (pThis->m_lstEntries.size())
+            rc = RTSemEventSignal(pThis->m_EventListComplete);
+            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"));
+
                 LogRel2(("Shared Clipboard: Waiting for transfer to complete ...\n"));
 
-                /* Transferring stuff can take a while, so don't use any timeout here. */
-                rc2 = RTSemEventWait(pThis->m_EventStatusChanged, RT_INDEFINITE_WAIT);
-                AssertRC(rc2);
-
-                switch (pThis->m_enmStatus)
+                for (;;)
                 {
-                    case Completed:
-                        LogRel2(("Shared Clipboard: Transfer complete\n"));
+                    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;
 
-                    case Canceled:
-                        LogRel2(("Shared Clipboard: Transfer canceled\n"));
-                        break;
-
-                    case Error:
-                        LogRel2(("Shared Clipboard: Transfer error occurred\n"));
-                        break;
-
-                    default:
-                        break;
+                    switch (pThis->m_enmStatus)
+                    {
+                        case Uninitialized: /* Can happen due to transfer erros. */
+                            LogRel2(("Shared Clipboard: Data object was unitialized\n"));
+                            break;
+
+                        case Initialized:
+                            AssertFailed(); /* State machine error -- debug this! */
+                            break;
+
+                        case Running:
+                            continue;
+
+                        case Completed:
+                            LogRel2(("Shared Clipboard: Transfer complete\n"));
+                            break;
+
+                        case Canceled:
+                            LogRel2(("Shared Clipboard: Transfer canceled\n"));
+                            break;
+
+                        case Error:
+                            LogRel(("Shared Clipboard: Transfer error within data object thread occurred\n"));
+                            break;
+
+                        default:
+                            AssertFailed();
+                            break;
+                    }
+
+                    break;
                 }
+
+                pThis->unlock();
             }
-            else
-               LogRel(("Shared Clipboard: No transfer root entries found -- should not happen, please file a bug report\n"));
-        }
-        else if (RT_FAILURE(rc))
-            LogRel(("Shared Clipboard: Transfer failed with %Rrc\n", rc));
-    }
+        }
+    }
+
+    if (RT_FAILURE(rc))
+        LogRel(("Shared Clipboard: Transfer failed with %Rrc\n", rc));
 
     LogFlowFuncLeaveRC(rc);
@@ -707 +767 @@
     int rc = VINF_SUCCESS;
 
-    if (   pFormatEtc->cfFormat == m_cfFileDescriptorA
+    /* 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
 #ifdef VBOX_CLIPBOARD_WITH_UNICODE_SUPPORT
-        || pFormatEtc->cfFormat == m_cfFileDescriptorW
+            || pFormatEtc->cfFormat == m_cfFileDescriptorW
 #endif
+           )
        )
     {
@@ -762 +828 @@
                 RT_NOREF(enmTransferStatus);
 
-                LogFlowFunc(("FormatIndex_FileDescriptor%s, enmTransferStatus=%s, m_fRunning=%RTbool\n",
-                             fUnicode ? "W" : "A", ShClTransferStatusToStr(enmTransferStatus), m_fThreadRunning));
+                LogFlowFunc(("FormatIndex_FileDescriptor%s, enmTransferStatus=%s\n",
+                             fUnicode ? "W" : "A", ShClTransferStatusToStr(enmTransferStatus)));
 
                 /* The caller can call GetData() several times, so make sure we don't do the same transfer multiple times. */
-                if (!m_fThreadRunning)
+                if (ShClTransferGetStatus(m_pTransfer) != SHCLTRANSFERSTATUS_STARTED)
                 {
                     /* Start the transfer + run it asynchronously in a separate thread. */
@@ -775 +841 @@
                         if (RT_SUCCESS(rc))
                         {
-                            m_fThreadRunning = true;
-
                             /* Leave lock while waiting. */
                             unlock();
@@ -782 +846 @@
                             /* 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"));
-                            rc = RTSemEventWait(m_EventListComplete, SHCL_TIMEOUT_DEFAULT_MS);
+                            rc = RTSemEventWait(m_EventListComplete, RT_MS_10SEC);
 
                             /* Re-acquire lock. */
                             lock();
+
+                            if (   m_pTransfer == NULL
+                                || m_enmStatus != Running) /* Still in running state? */
+                            {
+                                rc = VERR_OBJECT_DESTROYED;
+                                break;
+                            }
                         }
                     }
@@ -807 +878 @@
             }
 
+            case Completed:
+                break;
+
             default:
+                AssertFailedStmt(rc = VERR_STATE_CHANGED);
                 break;
         }
@@ -817 +892 @@
         }
     }
+
+    Log2Func(("enmStatus=%#x, pTransfer=%p, rc=%Rrc\n", m_enmStatus, m_pTransfer, rc));
 
     if (RT_SUCCESS(rc))
@@ -868 +945 @@
     unlock();
 
-    LogFlowFunc(("hr=%Rhrc\n", hr));
+    LogFlowFunc(("LEAVE hr=%Rhrc\n", hr));
     return hr;
 }
@@ -1039 +1116 @@
 int SharedClipboardWinDataObject::setTransferLocked(PSHCLTRANSFER pTransfer)
 {
+    AssertReturn(RTCritSectIsOwned(&m_CritSect), VERR_WRONG_ORDER);
+
     LogFlowFunc(("pTransfer=%p\n", pTransfer));
 
@@ -1056 +1135 @@
                 m_pTransfer = pTransfer;
 
+                SharedClipboardWinTransferCtx *pWinURITransferCtx = (SharedClipboardWinTransferCtx *)pTransfer->pvUser;
+                AssertPtr(pWinURITransferCtx);
+
+                pWinURITransferCtx->pDataObj = this; /* Save a backref to this object. */
+
                 ShClTransferAcquire(pTransfer);
             }
@@ -1066 +1150 @@
         if (m_pTransfer)
         {
+            SharedClipboardWinTransferCtx *pWinURITransferCtx = (SharedClipboardWinTransferCtx *)m_pTransfer->pvUser;
+            AssertPtr(pWinURITransferCtx);
+
+            pWinURITransferCtx->pDataObj = NULL; /* Release backref to this object. */
+
             ShClTransferRelease(m_pTransfer);
             m_pTransfer = NULL;
+
+            /* Make sure to notify any waiters. */
+            rc = RTSemEventSignal(m_EventListComplete);
+            AssertRC(rc);
         }
     }