Changeset 55457 in vbox for trunk/src/VBox/Main/src-server

Timestamp:

Apr 27, 2015 4:37:15 PM (10 years ago)

Author:

vboxsync

Message:

Main/Host: make sure that all relevant objects are uninitialized when the Host object is uninitialized, otherwise there will be races (especially when API clients crash) between the VirtualBox object shutdown and the other objects which might get cleaned up a little later, resulting in "pure virtual function" calls due to using the vtable of the VirtualBox object after it was freed

File:

• trunk/src/VBox/Main/src-server/HostImpl.cpp (modified) (5 diffs)

trunk/src/VBox/Main/src-server/HostImpl.cpp

@@ -476 +476 @@
      * Note that unregisterMetrics() has unregistered all metrics associated
      * with Host including network interface ones. We can destroy network
-     * interface objects now.
+     * interface objects now. Don't forget the uninit call, otherwise this
+     * causes a race with crashing API clients getting their stale references
+     * cleaned up and VirtualBox shutting down.
      */
-    m->llNetIfs.clear();
+    while (!m->llNetIfs.empty())
+    {
+        ComObjPtr<HostNetworkInterface> &pNet = m->llNetIfs.front();
+        pNet->uninit();
+        m->llNetIfs.pop_front();
+    }
 
 #ifdef VBOX_WITH_USB
@@ -493 +500 @@
 #ifdef VBOX_WITH_USB
     /* uninit all USB device filters still referenced by clients
-     * Note! HostUSBDeviceFilter::uninit() will modify llChildren. */
+     * Note! HostUSBDeviceFilter::uninit() will modify llChildren.
+     * This list should be already empty, but better be safe than sorry. */
     while (!m->llChildren.empty())
     {
         ComObjPtr<HostUSBDeviceFilter> &pChild = m->llChildren.front();
+        pChild->uninit();
         m->llChildren.pop_front();
-        pChild->uninit();
-    }
-
+    }
+
+    /* No need to uninit these, as either Machine::uninit() or the above loop
+     * already covered them all. Subset of llChildren. */
     m->llUSBDeviceFilters.clear();
 #endif
+
+    /* uninit all host DVD medium objects */
+    while (!m->llDVDDrives.empty())
+    {
+        ComObjPtr<Medium> &pMedium = m->llDVDDrives.front();
+        pMedium->uninit();
+        m->llDVDDrives.pop_front();
+    }
+    /* uninit all host floppy medium objects */
+    while (!m->llFloppyDrives.empty())
+    {
+        ComObjPtr<Medium> &pMedium = m->llFloppyDrives.front();
+        pMedium->uninit();
+        m->llFloppyDrives.pop_front();
+    }
 
     delete m;
@@ -1846 +1871 @@
                 }
                 if (!fFound)
+                {
+                    pCached->uninit();
                     itCached = pllCached->erase(itCached);
+                }
                 else
                     ++itCached;
@@ -2990 +3018 @@
             {
                 fGone = false;
+                (*itNew)->uninit();
                 listCopy.erase(itNew);
                 break;
@@ -2998 +3027 @@
 # ifdef VBOX_WITH_RESOURCE_USAGE_API
             (*itOld)->i_unregisterMetrics(aCollector, this);
+            (*itOld)->uninit();
 # endif
         }