Changeset 92866 in vbox

Timestamp:

Dec 10, 2021 2:51:11 PM (3 years ago)

Author:

vboxsync

Message:

Guest Control/Main: When processing service callback messages for guest sessions, do the actual (write) locking much further down in GuestSession::i_setSessionStatus(), so that it can release the write lock right before firing off GuestSessionStateChanged events. This is needed in order to make session ID lookups via IGuestSessionStateChangedEvent::Id getter possible (takes the read lock).

File:

• trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp (modified) (8 diffs)

trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp

@@ -1315 +1315 @@
 }
 
-int GuestSession::i_dispatchToThis(PVBOXGUESTCTRLHOSTCBCTX pCbCtx, PVBOXGUESTCTRLHOSTCALLBACK pSvcCb)
+/**
+ * Main handler for guest session messages from the guest.
+ *
+ * @returns VBox status code.
+ * @param   pCbCtx              Host callback context from HGCM service.
+ * @param   pSvcCbData          HGCM service callback data.
+ *
+ * @note    No locking!
+ */
+int GuestSession::i_dispatchToThis(PVBOXGUESTCTRLHOSTCBCTX pCbCtx, PVBOXGUESTCTRLHOSTCALLBACK pSvcCbData)
 {
     AssertPtrReturn(pCbCtx, VERR_INVALID_POINTER);
-    AssertPtrReturn(pSvcCb, VERR_INVALID_POINTER);
-
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
+    AssertPtrReturn(pSvcCbData, VERR_INVALID_POINTER);
 
     LogFlowThisFunc(("sessionID=%RU32, CID=%RU32, uMessage=%RU32, pSvcCb=%p\n",
-                     mData.mSession.mID, pCbCtx->uContextID, pCbCtx->uMessage, pSvcCb));
+                     mData.mSession.mID, pCbCtx->uContextID, pCbCtx->uMessage, pSvcCbData));
     int rc;
     switch (pCbCtx->uMessage)
@@ -1334 +1341 @@
         case GUEST_MSG_SESSION_NOTIFY: /* Guest Additions >= 4.3.0. */
         {
-            rc = i_onSessionStatusChange(pCbCtx, pSvcCb);
+            rc = i_onSessionStatusChange(pCbCtx, pSvcCbData);
             break;
         }
 
         default:
-            rc = dispatchGeneric(pCbCtx, pSvcCb);
+            rc = dispatchGeneric(pCbCtx, pSvcCbData);
             break;
     }
@@ -1834 +1841 @@
 }
 
-/** No locking! */
+/**
+ * Handles guest session status changes from the guest.
+ *
+ * @returns VBox status code.
+ * @param   pCbCtx              Host callback context from HGCM service.
+ * @param   pSvcCbData          HGCM service callback data.
+ *
+ * @note    Takes the read lock (for session ID lookup).
+ */
 int GuestSession::i_onSessionStatusChange(PVBOXGUESTCTRLHOSTCBCTX pCbCtx, PVBOXGUESTCTRLHOSTCALLBACK pSvcCbData)
 {
@@ -1850 +1865 @@
     vrc = HGCMSvcGetU32(&pSvcCbData->mpaParms[2], &dataCb.uResult);
     AssertRCReturn(vrc, vrc);
+
+    AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     LogFlowThisFunc(("ID=%RU32, uType=%RU32, rcGuest=%Rrc\n",
@@ -1916 +1933 @@
     }
 
+    /* Leave the lock, as i_setSessionStatus() below will require a write lock for actually
+     * committing the session state. */
+    alock.release();
+
     if (RT_SUCCESS(vrc))
     {
@@ -2643 +2664 @@
 }
 
-/* Does not do locking; caller is responsible for that! */
+/**
+ * Sets the guest session's current status.
+ *
+ * @returns VBox status code.
+ * @param   sessionStatus       Session status to set.
+ * @param   sessionRc           Session result to set (for error handling).
+ *
+ * @note    Takes the write lock.
+ */
 int GuestSession::i_setSessionStatus(GuestSessionStatus_T sessionStatus, int sessionRc)
 {
@@ -2658 +2687 @@
     else
         AssertMsg(RT_SUCCESS(sessionRc), ("Guest rc must not be an error (%Rrc)\n", sessionRc));
+
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     int vrc = VINF_SUCCESS;
@@ -2676 +2707 @@
                                     i_guestErrorToString(sessionRc));
         AssertRC(rc2);
+
+        alock.release(); /* Release lock before firing off event. */
 
         ::FireGuestSessionStateChangedEvent(mEventSource, this, mData.mSession.mID, sessionStatus, errorInfo);