Changeset 75574 in vbox for trunk/src/VBox/Main/src-client

Timestamp:

Nov 19, 2018 2:31:44 PM (6 years ago)

Author:

vboxsync

Message:

Main/HGCM: Shutdown HGCM on VM construction failure, indicating that pUVM is invalid preventing info and stats deregistration calls.

Location:

trunk/src/VBox/Main/src-client

Files:

• ConsoleImpl.cpp (modified) (1 diff)
• HGCM.cpp (modified) (11 diffs)
• VMMDevInterface.cpp (modified) (4 diffs)

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

@@ -10396 +10396 @@
              * If VMR3Create() failed it has released the VM memory.
              */
+            if (pConsole->m_pVMMDev)
+            {
+                alock.release(); /* just to be on the safe side... */
+                pConsole->m_pVMMDev->hgcmShutdown(true /*fUvmIsInvalid*/);
+                alock.acquire();
+            }
             VMR3ReleaseUVM(pConsole->mpUVM);
             pConsole->mpUVM = NULL;

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

@@ -156 +156 @@
          */
         static int LoadService(const char *pszServiceLibrary, const char *pszServiceName, PUVM pUVM);
-        void UnloadService(void);
-
-        static void UnloadAll(void);
+        void UnloadService(bool fUvmIsInvalid);
+
+        static void UnloadAll(bool fUvmIsInvalid);
 
         static int ResolveService(HGCMService **ppsvc, const char *pszServiceName);
@@ -865 +865 @@
      AssertPtrReturn(pService, VERR_INVALID_PARAMETER);
 
-     return STAMR3DeregisterV(pService->m_pUVM, pszPatFmt, va);
+     if (pService->m_pUVM)
+         return STAMR3DeregisterV(pService->m_pUVM, pszPatFmt, va);
+     return VINF_SUCCESS;
 }
 
@@ -887 +889 @@
      HGCMService *pService = static_cast <HGCMService *>(pvInstance);
      AssertPtrReturn(pService, VERR_INVALID_PARAMETER);
-
-     return DBGFR3InfoDeregisterExternal(pService->m_pUVM, pszName);
+     if (pService->m_pUVM)
+         return DBGFR3InfoDeregisterExternal(pService->m_pUVM, pszName);
+     return VINF_SUCCESS;
 }
 
@@ -994 +997 @@
     }
 
-    if (m_pszSvcName)
+    if (m_pszSvcName && m_pUVM)
         STAMR3DeregisterF(m_pUVM, "/HGCM/%s/*", m_pszSvcName);
     m_pUVM = NULL;
@@ -1121 +1124 @@
  * @thread main HGCM
  */
-void HGCMService::UnloadService(void)
+void HGCMService::UnloadService(bool fUvmIsInvalid)
 {
     LogFlowFunc(("name = %s\n", m_pszSvcName));
+
+    if (fUvmIsInvalid)
+        m_pUVM = NULL;
 
     /* Remove the service from the list. */
@@ -1158 +1164 @@
  * @thread main HGCM
  */
-/* static */ void HGCMService::UnloadAll(void)
+/* static */ void HGCMService::UnloadAll(bool fUvmIsInvalid)
 {
     while (sm_pSvcListHead)
     {
-        sm_pSvcListHead->UnloadService();
+        sm_pSvcListHead->UnloadService(fUvmIsInvalid);
     }
 }
@@ -1844 +1850 @@
 class HGCMMsgMainQuit: public HGCMMsgCore
 {
+    public:
+        /** Whether UVM has gone invalid already or not. */
+        bool fUvmIsInvalid;
 };
 
@@ -1849 +1858 @@
 {
     public:
-        /* Returned handle to be used in HGCMMsgMainUnregisterExtension. */
+        /** Returned handle to be used in HGCMMsgMainUnregisterExtension. */
         HGCMSVCEXTHANDLE *pHandle;
-        /* Name of the service. */
+        /** Name of the service. */
         const char *pszServiceName;
-        /* The extension entry point. */
+        /** The extension entry point. */
         PFNHGCMSVCEXT pfnExtension;
-        /* The extension pointer. */
+        /** The extension pointer. */
         void *pvExtension;
 };
@@ -2079 +2088 @@
             case HGCM_MSG_QUIT:
             {
+                HGCMMsgMainQuit *pMsg = (HGCMMsgMainQuit *)pMsgCore;
                 LogFlowFunc(("HGCM_MSG_QUIT\n"));
 
-                HGCMService::UnloadAll();
+                HGCMService::UnloadAll(pMsg->fUvmIsInvalid);
 
                 fQuit = true;
@@ -2636 +2646 @@
 }
 
-int HGCMHostShutdown(void)
+int HGCMHostShutdown(bool fUvmIsInvalid /*= false*/)
 {
     LogFlowFunc(("\n"));
@@ -2649 +2659 @@
     {
         /* Send the quit message to the main hgcmThread. */
-        HGCMMsgCore *pMsg;
-        rc = hgcmMsgAlloc(g_pHgcmThread, &pMsg, HGCM_MSG_QUIT, hgcmMainMessageAlloc);
+        HGCMMsgCore *pMsgCore;
+        rc = hgcmMsgAlloc(g_pHgcmThread, &pMsgCore, HGCM_MSG_QUIT, hgcmMainMessageAlloc);
 
         if (RT_SUCCESS(rc))
         {
+            HGCMMsgMainQuit *pMsg = (HGCMMsgMainQuit *)pMsgCore;
+            pMsg->fUvmIsInvalid = fUvmIsInvalid;
+
             rc = hgcmMsgSend(pMsg);
 

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

@@ -59 +59 @@
 {
     /** Pointer to the VMMDev object. */
-    VMMDev                      *pVMMDev;
+    VMMDev                     *pVMMDev;
     /** Pointer to the driver instance structure. */
     PPDMDRVINS                  pDrvIns;
@@ -95 +95 @@
 {
 #ifdef VBOX_WITH_HGCM
-    if (hgcmIsActive())
-    {
-        ASMAtomicWriteBool(&m_fHGCMActive, false);
-        HGCMHostShutdown();
-    }
-#endif /* VBOX_WITH_HGCM */
+    if (ASMAtomicCmpXchgBool(&m_fHGCMActive, false, true))
+        HGCMHostShutdown(true /*fUvmIsInvalid*/);
+#endif
     RTSemEventDestroy(mCredentialsEvent);
     if (mpDrv)
@@ -707 +704 @@
 }
 
-void VMMDev::hgcmShutdown(void)
-{
-    ASMAtomicWriteBool(&m_fHGCMActive, false);
-    HGCMHostShutdown();
+/**
+ * Used by Console::i_powerDown to shut down the services before the VM is destroyed.
+ */
+void VMMDev::hgcmShutdown(bool fUvmIsInvalid /*= false*/)
+{
+    if (ASMAtomicCmpXchgBool(&m_fHGCMActive, false, true))
+        HGCMHostShutdown(fUvmIsInvalid);
 }
 
@@ -777 +777 @@
     LogFlow(("VMMDev::drvDestruct: iInstance=%d\n", pDrvIns->iInstance));
 
-#ifdef VBOX_WITH_HGCM
-    /* HGCM is shut down on the VMMDev destructor. */
-#endif /* VBOX_WITH_HGCM */
     if (pThis->pVMMDev)
+    {
+#ifdef VBOX_WITH_HGCM
+        /* When VM construction goes wrong, we prefer shutting down HGCM here
+           while pUVM is still valid, rather than in ~VMMDev. */
+        if (ASMAtomicCmpXchgBool(&pThis->pVMMDev->m_fHGCMActive, false, true))
+            HGCMHostShutdown();
+#endif
         pThis->pVMMDev->mpDrv = NULL;
+    }
 }