Changeset 91097 in vbox

Timestamp:

Sep 2, 2021 2:42:26 PM (3 years ago)

Author:

vboxsync

Message:

HGCM: Put back pfnDisconnectClient. oem2ticketref:46

Location:

trunk

Files:

• include/VBox/hgcmsvc.h (modified) (2 diffs)
• src/VBox/Main/src-client/HGCM.cpp (modified) (7 diffs)

trunk/include/VBox/hgcmsvc.h

@@ -83 +83 @@
  * 8.1->9.1 Because pfnDisconnectClient was (temporarily) removed, and
  *          acMaxClients and acMaxCallsPerClient added (VBox 6.1.26).
+ * 9.1->10.1 Because pfnDisconnectClient was added back (VBox 6.1.28).
  */
 #define VBOX_HGCM_SVC_VERSION_MAJOR (0x0009)
@@ -101 +102 @@
     void *pvInstance;
 
-#if 0 /* Not thread safe. */
-    /** The service disconnects the client. */
-    DECLR3CALLBACKMEMBER(void, pfnDisconnectClient, (void *pvInstance, uint32_t u32ClientID));
-#endif
+    /**
+     * The service disconnects the client.
+     *
+     * This can only be used during VBOXHGCMSVCFNTABLE::pfnConnect or
+     * VBOXHGCMSVCFNTABLE::pfnDisconnect and will fail if called out side that
+     * context.
+     *
+     * There will be no VBOXHGCMSVCFNTABLE::pfnDisconnect callback for a client
+     * diconnected in this manner.
+     *
+     * @returns VBox status code.
+     * @retval  VERR_NOT_FOUND if the client ID was not found.
+     * @retval  VERR_INVALID_CONTEXT if not called during connect or disconnect.
+     *
+     * @remarks Used by external parties, so don't remove just because we don't use
+     *          it ourselves.
+     */
+    DECLR3CALLBACKMEMBER(int, pfnDisconnectClient, (void *pvInstance, uint32_t u32ClientID));
 
     /**

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

@@ -109 +109 @@
         VBOXHGCMSVCFNTABLE m_fntable;
 
+        /** Set if servicing SVC_MSG_CONNECT or SVC_MSG_DISCONNECT.
+         * Used for context checking pfnDisconnectClient calls, as it can only
+         * safely be made when the main HGCM thread is waiting on the service to
+         * process those messages. */
+        bool m_fInConnectOrDisconnect;
+
         uint32_t m_acClients[HGCM_CLIENT_CATEGORY_MAX]; /**< Clients per category. */
         uint32_t m_cClients;
@@ -143 +149 @@
 
         static DECLCALLBACK(int)  svcHlpCallComplete(VBOXHGCMCALLHANDLE callHandle, int32_t rc);
-        static DECLCALLBACK(void) svcHlpDisconnectClient(void *pvInstance, uint32_t u32ClientId);
+        static DECLCALLBACK(int)  svcHlpDisconnectClient(void *pvInstance, uint32_t u32ClientId);
         static DECLCALLBACK(bool) svcHlpIsCallRestored(VBOXHGCMCALLHANDLE callHandle);
         static DECLCALLBACK(bool) svcHlpIsCallCancelled(VBOXHGCMCALLHANDLE callHandle);
@@ -282 +288 @@
     m_hLdrMod    (NIL_RTLDRMOD),
     m_pfnLoad    (NULL),
+    m_fInConnectOrDisconnect(false),
     m_cClients   (0),
     m_cClientsAllocated (0),
@@ -672 +679 @@
                 if (pClient)
                 {
+                    pSvc->m_fInConnectOrDisconnect = true;
                     rc = pSvc->m_fntable.pfnConnect(pSvc->m_fntable.pvService, pMsg->u32ClientId,
                                                     HGCM_CLIENT_DATA(pSvc, pClient),
                                                     pMsg->fRequestor, pMsg->fRestoring);
+                    pSvc->m_fInConnectOrDisconnect = false;
 
                     hgcmObjDereference(pClient);
@@ -692 +701 @@
                 if (pMsg->pClient)
                 {
+                    pSvc->m_fInConnectOrDisconnect = true;
                     rc = pSvc->m_fntable.pfnDisconnect(pSvc->m_fntable.pvService, pMsg->u32ClientId,
                                                        HGCM_CLIENT_DATA(pSvc, pMsg->pClient));
+                    pSvc->m_fInConnectOrDisconnect = false;
                 }
                 else
@@ -913 +924 @@
 }
 
-#if 0 /* not thread safe */
 /**
  * @interface_method_impl{VBOXHGCMSVCHELPERS,pfnDisconnectClient}
  */
-/* static */ DECLCALLBACK(void) HGCMService::svcHlpDisconnectClient(void *pvInstance, uint32_t u32ClientId)
+/* static */ DECLCALLBACK(int) HGCMService::svcHlpDisconnectClient(void *pvInstance, uint32_t u32ClientId)
 {
      HGCMService *pService = static_cast <HGCMService *> (pvInstance);
-
-     AssertMsgFailed(("This is unused code with a serialization issue.\n"
-                      "It touches data which is normally serialized by only running on the HGCM thread!\n"));
-
-     if (pService)
-     {
-         pService->DisconnectClient(u32ClientId, true, NULL);
-     }
-}
-#endif
+     AssertReturn(pService, VERR_INVALID_HANDLE);
+
+     /* Only safe to call when the main HGCM thread is waiting on the service
+        to handle a SVC_MSG_CONNECT or SVC_MSG_DISCONNECT message.  Otherwise
+        we'd risk racing it and corrupt data structures. */
+     AssertReturn(pService->m_fInConnectOrDisconnect, VERR_INVALID_CONTEXT);
+
+     return pService->DisconnectClient(u32ClientId, true, NULL);
+}
 
 /**
@@ -1135 +1144 @@
             m_svcHelpers.pfnCallComplete       = svcHlpCallComplete;
             m_svcHelpers.pvInstance            = this;
-#if 0 /* not thread safe */
             m_svcHelpers.pfnDisconnectClient   = svcHlpDisconnectClient;
-#endif
             m_svcHelpers.pfnIsCallRestored     = svcHlpIsCallRestored;
             m_svcHelpers.pfnIsCallCancelled    = svcHlpIsCallCancelled;