Changeset 39312 in vbox for trunk/src/VBox/HostServices/GuestControl/service.cpp

Timestamp:

Nov 15, 2011 2:40:49 PM (13 years ago)

Author:

vboxsync

Message:

HostServices/GuestCtrl: Fixed cancellation of buffered (unassigned) host commands or assigned commands on client disconnect, logging adjustments.

File:

• trunk/src/VBox/HostServices/GuestControl/service.cpp (modified) (18 diffs)

trunk/src/VBox/HostServices/GuestControl/service.cpp

@@ -27 +27 @@
  * - Client: A client (e.g. VBoxService) running inside the guest OS waiting for
  *           new host commands to perform. There can be multiple clients connected
- *           to a service. A client is represented by its HGCM client ID.
+ *           to this service. A client is represented by its unique HGCM client ID.
  * - Context ID: An (almost) unique ID automatically generated on the host (Main API)
  *               to not only distinguish clients but individual requests. Because
@@ -285 +285 @@
     int clientConnect(uint32_t u32ClientID, void *pvClient);
     int clientDisconnect(uint32_t u32ClientID, void *pvClient);
-    int sendHostCmdToGuest(HostCmd *pCmd, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
+    int assignHostCmdToGuest(HostCmd *pCmd, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
     int retrieveNextHostCmd(uint32_t u32ClientID, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
+    int cancelHostCmd(uint32_t u32ContextID);
     int cancelPendingWaits(uint32_t u32ClientID);
     int notifyHost(uint32_t eFunction, uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
@@ -492 +493 @@
 int Service::clientDisconnect(uint32_t u32ClientID, void *pvClient)
 {
-    LogFlowFunc(("Client (%ld) disconnected\n", u32ClientID));
+    LogFlowFunc(("Client (ID=%u, %u clients total) disconnected\n",
+                 u32ClientID, mNumClients));
     Assert(mNumClients > 0);
     mNumClients--;
+
+    /* If this was the last connected (guest) client we need to
+     * unblock all eventually queued up (waiting) host calls. */
+    bool fAllClientsDisconnected = mNumClients == 0;
+    if (fAllClientsDisconnected)
+        LogFlowFunc(("No connected clients left, notifying all queued up callbacks\n"));
 
     /*
@@ -516 +524 @@
            && RT_SUCCESS(rc))
     {
-        if (it->mClientID == u32ClientID)
+        /*
+         * Unblock/call back all queued items of the specified client
+         * or for all items in case there is no waiting client around
+         * anymore.
+         */
+        if (   it->mClientID == u32ClientID
+            || fAllClientsDisconnected)
         {
             std::list< uint32_t >::iterator itContext = it->mContextList.begin();
-            while (   itContext != it->mContextList.end()
-                   && RT_SUCCESS(rc))
+            while (itContext != it->mContextList.end())
             {
-                LogFlowFunc(("Notifying host context %u of disconnect ...\n", (*itContext)));
+                uint32_t uContextID = (*itContext);
 
                 /*
@@ -528 +541 @@
                  * around and need to be cleaned up (canceling waits etc).
                  */
-                if (mpfnHostCallback)
+                LogFlowFunc(("Notifying CID=%u of disconnect ...\n", uContextID));
+                rc = cancelHostCmd(uContextID);
+                if (RT_FAILURE(rc))
                 {
-                    CALLBACKDATACLIENTDISCONNECTED data;
-                    data.hdr.u32Magic = CALLBACKDATAMAGIC_CLIENT_DISCONNECTED;
-                    data.hdr.u32ContextID = (*itContext);
-                    rc = mpfnHostCallback(mpvHostData, GUEST_DISCONNECTED, (void *)(&data), sizeof(data));
-                    if (RT_FAILURE(rc))
-                        LogFlowFunc(("Notification of host context %u failed with %Rrc\n", rc));
+                    LogFlowFunc(("Cancelling of CID=%u failed with rc=%Rrc\n",
+                                 uContextID, rc));
+                    /* Keep going. */
                 }
+
                 itContext++;
             }
@@ -544 +557 @@
             it++;
     }
+
+    if (fAllClientsDisconnected)
+    {
+        /*
+         * If all clients disconnected we also need to make sure that all buffered
+         * host commands need to be notified, because Main is waiting a notification
+         * via a (multi stage) progress object.
+         */
+        HostCmdListIter it;
+        for (it = mHostCmds.begin(); it != mHostCmds.end(); it++)
+        {
+            rc = cancelHostCmd(it->mContextID);
+            if (RT_FAILURE(rc))
+            {
+                LogFlowFunc(("Cancelling of buffered CID=%u failed with rc=%Rrc\n",
+                             it->mContextID, rc));
+                /* Keep going. */
+            }
+
+            paramBufferFree(&it->mParmBuf);
+        }
+
+        mHostCmds.clear();
+    }
+
     return rc;
 }
 
 /**
- * Sends a specified host command to a client.
+ * Assigns a specified host command to a client.
  *
  * @return  IPRT status code.
@@ -556 +594 @@
  * @param   paParms                 Array of parameters.
  */
-int Service::sendHostCmdToGuest(HostCmd *pCmd, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
-{
-    AssertPtr(pCmd);
+int Service::assignHostCmdToGuest(HostCmd *pCmd, VBOXHGCMCALLHANDLE callHandle, uint32_t cParms, VBOXHGCMSVCPARM paParms[])
+{
+    AssertPtrReturn(pCmd, VERR_INVALID_POINTER);
     int rc;
 
-    /* Sufficient parameter space? */
+    /* Does the current host command need more parameter space which
+     * the client does not provide yet? */
     if (pCmd->mParmBuf.uParmCount > cParms)
     {
@@ -578 +617 @@
         rc = paramBufferAssign(&pCmd->mParmBuf, cParms, paParms);
     }
+
+    LogFlowFunc(("Returned with rc=%Rrc\n", rc));
     return rc;
 }
@@ -632 +673 @@
          */
          HostCmd curCmd = mHostCmds.front();
-         rc = sendHostCmdToGuest(&curCmd, callHandle, cParms, paParms);
+         rc = assignHostCmdToGuest(&curCmd, callHandle, cParms, paParms);
          if (RT_SUCCESS(rc))
          {
@@ -658 +699 @@
          else
          {
-            /* Client did not understand the message or something else weird happened. Try again one
-             * more time and drop it if it didn't get handled then. */
+             /* Client did not understand the message or something else weird happened. Try again one
+              * more time and drop it if it didn't get handled then. */
              if (++curCmd.mTries > 1)
              {
@@ -668 +709 @@
     }
     return rc;
+}
+
+/**
+ * Cancels a buffered host command to unblock waits on Main side
+ * (via (multi stage) progress objects.
+ *
+ * @return  IPRT status code.
+ * @param   u32ContextID                Context ID of host command to cancel.
+ */
+int Service::cancelHostCmd(uint32_t u32ContextID)
+{
+    AssertReturn(u32ContextID, VERR_INVALID_PARAMETER);
+    Assert(mpfnHostCallback);
+
+    LogFlowFunc(("Cancelling CID=%u ...\n", u32ContextID));
+
+    CALLBACKDATACLIENTDISCONNECTED data;
+    data.hdr.u32Magic = CALLBACKDATAMAGIC_CLIENT_DISCONNECTED;
+    data.hdr.u32ContextID = u32ContextID;
+
+    AssertPtr(mpfnHostCallback);
+    AssertPtr(mpvHostData);
+
+    return mpfnHostCallback(mpvHostData, GUEST_DISCONNECTED, (void *)(&data), sizeof(data));
 }
 
@@ -789 +854 @@
     HostCmd newCmd;
     int rc = paramBufferAllocate(&newCmd.mParmBuf, eFunction, cParms, paParms);
-    if (RT_SUCCESS(rc) && cParms)
+    if (   RT_SUCCESS(rc)
+        && cParms) /* Make sure we at least get one parameter (that is, the context ID). */
     {
         /*
@@ -798 +864 @@
         Assert(newCmd.mContextID > 0);
     }
+    else if (!cParms)
+        rc = VERR_INVALID_PARAMETER;
 
     if (RT_SUCCESS(rc))
     {
+        LogFlowFunc(("Handling host command CID = %u\n",
+                     newCmd.mContextID));
+
         bool fProcessed = false;
 
@@ -807 +878 @@
         {
             ClientWaiter guest = mClientWaiterList.front();
-            rc = sendHostCmdToGuest(&newCmd,
-                                    guest.mHandle, guest.mNumParms, guest.mParms);
+            rc = assignHostCmdToGuest(&newCmd,
+                                      guest.mHandle, guest.mNumParms, guest.mParms);
 
             /* In any case the client did something, so wake up and remove from list. */
@@ -825 +896 @@
             else /* If command was understood by the client, free and remove from host commands list. */
             {
+                LogFlowFunc(("Host command CID = %u processed with rc=%Rrc\n",
+                             newCmd.mContextID, rc));
+
                 paramBufferFree(&newCmd.mParmBuf);
-                fProcessed = true;
             }
         }
 
-        /* If not processed, buffer it ... */
         if (!fProcessed)
         {
+            LogFlowFunc(("Buffering host command CID = %u (rc=%Rrc)\n",
+                         newCmd.mContextID, rc));
+
             mHostCmds.push_back(newCmd);
-#if 0
-            /* Limit list size by deleting oldest element. */
-            if (mHostCmds.size() > 256) /** @todo Use a define! */
-                mHostCmds.pop_front();
-#endif
-        }
-    }
+        }
+    }
+
+    LogFlowFunc(("Returned with rc=%Rrc\n", rc));
     return rc;
 }
@@ -858 +930 @@
 {
     int rc = VINF_SUCCESS;
-    LogFlowFunc(("u32ClientID = %d, fn = %d, cParms = %d, pparms = %d\n",
+    LogFlowFunc(("u32ClientID = %u, fn = %u, cParms = %u, paParms = 0x%p\n",
                  u32ClientID, eFunction, cParms, paParms));
     try
@@ -913 +985 @@
 {
     int rc = VERR_NOT_SUPPORTED;
-    LogFlowFunc(("fn = %d, cParms = %d, pparms = %d\n",
+    LogFlowFunc(("fn = %u, cParms = %u, paParms = 0x%p\n",
                  eFunction, cParms, paParms));
     try
@@ -930 +1002 @@
 int Service::uninit()
 {
-    /* Free allocated buffered host commands. */
-    HostCmdListIter it;
-    for (it = mHostCmds.begin(); it != mHostCmds.end(); it++)
-        paramBufferFree(&it->mParmBuf);
-    mHostCmds.clear();
+    Assert(mHostCmds.empty());
 
     return VINF_SUCCESS;