Changeset 48906 in vbox for trunk

Timestamp:

Oct 5, 2013 10:39:42 PM (11 years ago)

Author:

vboxsync

Message:

GUI: Prevent cleanup race in VBoxGlobal media-enumerator-mess from crashing again during termination.

Location:

trunk/src/VBox/Frontends/VirtualBox/src

Files:

• globals/VBoxGlobal.cpp (modified) (10 diffs)
• globals/VBoxGlobal.h (modified) (6 diffs)
• medium/Makefile.kup (added)
• medium/UIMediumEnumerator.cpp (modified) (4 diffs)
• medium/UIMediumEnumerator.h (modified) (3 diffs)
• selector/graphics/details/UIGDetailsElements.cpp (modified) (1 diff)

trunk/src/VBox/Frontends/VirtualBox/src/globals/VBoxGlobal.cpp

@@ -25 +25 @@
 #include <QDesktopServices>
 #include <QMutex>
+#include <QReadLocker>
 #include <QToolButton>
 #include <QProcess>
@@ -943 +944 @@
 
         /* Search for corresponding UI medium again: */
+
         uimedium = medium(strMediumID);
         if (uimedium.isNull())
@@ -1664 +1666 @@
 {
     /* Create medium in medium-enumerator: */
-    m_pMediumEnumerator->createMedium(medium);
+    QReadLocker cleanupRacePreventor(&m_mediumEnumeratorDtorRwLock);
+    if (m_pMediumEnumerator)
+        m_pMediumEnumerator->createMedium(medium);
 }
 
@@ -1670 +1674 @@
 {
     /* Update medium of medium-enumerator: */
-    m_pMediumEnumerator->updateMedium(medium);
+    QReadLocker cleanupRacePreventor(&m_mediumEnumeratorDtorRwLock);
+    if (m_pMediumEnumerator)
+        m_pMediumEnumerator->updateMedium(medium);
 }
 
@@ -1676 +1682 @@
 {
     /* Delete medium from medium-enumerator: */
-    m_pMediumEnumerator->deleteMedium(strMediumID);
+    QReadLocker cleanupRacePreventor(&m_mediumEnumeratorDtorRwLock);
+    if (m_pMediumEnumerator)
+        m_pMediumEnumerator->deleteMedium(strMediumID);
 }
 
@@ -1841 +1849 @@
 
     /* Redirect request to medium-enumerator: */
-    m_pMediumEnumerator->enumerateMediums();
+    QReadLocker cleanupRacePreventor(&m_mediumEnumeratorDtorRwLock);
+    if (m_pMediumEnumerator)
+        m_pMediumEnumerator->enumerateMediums();
 }
 
@@ -1847 +1857 @@
 {
     /* Redirect request to medium-enumerator: */
-    return m_pMediumEnumerator->isMediumEnumerationInProgress();
+    return m_pMediumEnumerator
+        && m_pMediumEnumerator->isMediumEnumerationInProgress();
 }
 
@@ -1853 +1864 @@
 {
     /* Redirect call to medium-enumerator: */
-    return m_pMediumEnumerator->medium(strMediumID);
+    QReadLocker cleanupRacePreventor(&m_mediumEnumeratorDtorRwLock);
+    if (m_pMediumEnumerator)
+        return m_pMediumEnumerator->medium(strMediumID);
+    return UIMedium();
 }
 
@@ -1859 +1873 @@
 {
     /* Redirect call to medium-enumerator: */
-    return m_pMediumEnumerator->mediumIDs();
+    QReadLocker cleanupRacePreventor(&m_mediumEnumeratorDtorRwLock);
+    if (m_pMediumEnumerator)
+        return m_pMediumEnumerator->mediumIDs();
+    return QList<QString>();
 }
 
@@ -4352 +4369 @@
 
     /* Cleanup medium-enumerator: */
+    m_mediumEnumeratorDtorRwLock.lockForWrite();
     delete m_pMediumEnumerator;
     m_pMediumEnumerator = 0;
+    m_mediumEnumeratorDtorRwLock.unlock();
 
     if (mSelectorWnd)

trunk/src/VBox/Frontends/VirtualBox/src/globals/VBoxGlobal.h

@@ -1 +1 @@
 /** @file
- *
- * VBox frontends: Qt GUI ("VirtualBox"):
- * VBoxGlobal class declaration
+ * VBox Qt GUI - VBoxGlobal class declaration.
  */
 
@@ -17 +15 @@
  */
 
-#ifndef __VBoxGlobal_h__
-#define __VBoxGlobal_h__
+#ifndef ___VBoxGlobal_h___
+#define ___VBoxGlobal_h___
 
 /* Qt includes: */
@@ -27 +25 @@
 #include <QHash>
 #include <QFileIconProvider>
+#include <QReadWriteLock>
 #ifdef Q_WS_MAC
 # include <QSet>
@@ -73 +72 @@
 
     bool isValid() { return mValid; }
+    bool isCleaningUp() { return m_sfCleanupInProgress; }
 
     static QString qtRTVersionString();
@@ -429 +429 @@
     /* Variable: Medium-enumeration stuff: */
     UIMediumEnumerator *m_pMediumEnumerator;
+    mutable QReadWriteLock m_mediumEnumeratorDtorRwLock;
 
     RenderMode vm_render_mode;
@@ -509 +510 @@
 inline VBoxGlobal& vboxGlobal() { return *VBoxGlobal::instance(); }
 
-#endif /* __VBoxGlobal_h__ */
-
+#endif /* !___VBoxGlobal_h___ */
+

trunk/src/VBox/Frontends/VirtualBox/src/medium/UIMediumEnumerator.cpp

@@ -1 +1 @@
 /* $Id$ */
 /** @file
- *
- * VBox frontends: Qt GUI ("VirtualBox"):
- * UIMediumEnumerator class implementation
+ * VBox Qt GUI - UIMediumEnumerator class implementation.
  */
 
@@ -166 +164 @@
     addMediumsToMap(vboxGlobal().host().GetFloppyDrives(), mediums, UIMediumType_Floppy);
     addMediumsToMap(vboxGlobal().virtualBox().GetFloppyImages(), mediums, UIMediumType_Floppy);
+    if (vboxGlobal().isCleaningUp())
+        return; /* VBoxGlobal is cleaning up, abort immediately. */
     m_mediums = mediums;
 
@@ -316 +316 @@
     foreach (CMedium medium, inputMediums)
     {
+        /* If VBoxGlobal is cleaning up, abort immediately: */
+        if (vboxGlobal().isCleaningUp())
+            break;
+
         /* Prepare uimedium on the basis of current medium: */
         QString strMediumID = medium.GetId();
@@ -332 +336 @@
     foreach (CMedium medium, inputMediums)
     {
+        /* If VBoxGlobal is cleaning up, abort immediately: */
+        if (vboxGlobal().isCleaningUp())
+            break;
+
         /* Prepare uimedium on the basis of current medium: */
         QString strMediumID = medium.GetId();

trunk/src/VBox/Frontends/VirtualBox/src/medium/UIMediumEnumerator.h

@@ -1 +1 @@
 /** @file
- *
- * VBox frontends: Qt GUI ("VirtualBox"):
- * UIMediumEnumerator class declaration
+ * VBox Qt GUI - UIMediumEnumerator class declaration.
  */
 
@@ -17 +15 @@
  */
 
-#ifndef __UIMediumEnumerator_h__
-#define __UIMediumEnumerator_h__
+#ifndef ___UIMediumEnumerator_h___
+#define ___UIMediumEnumerator_h___
 
 /* Qt includes: */
@@ -88 +86 @@
 };
 
-#endif /* __UIMediumEnumerator_h__ */
+#endif /* !___UIMediumEnumerator_h___ */
+

trunk/src/VBox/Frontends/VirtualBox/src/selector/graphics/details/UIGDetailsElements.cpp

@@ -62 +62 @@
 UIGDetailsElementInterface::~UIGDetailsElementInterface()
 {
+    /* VBoxGlobal must still be valid at this point, otherwise our thread might
+       be doing really bad things to already deleted memory! */
+    Assert(vboxGlobal().isValid());
+    Assert(!vboxGlobal().isCleaningUp());
+
     cleanupThread();
 }