Changeset 911 in vbox

Timestamp:

Feb 14, 2007 6:41:37 PM (18 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

18636

Message:

Main: Fixed an assertion hit when opening the VDM dialog from the console window (for example, to mount a CD media) of a running VM or doing any other media accessibility check on it that could leave to undefined behavior in the release builds.

Location:

trunk/src/VBox/Main

Files:

• HardDiskImpl.cpp (modified) (3 diffs)
• include/HardDiskImpl.h (modified) (1 diff)

trunk/src/VBox/Main/HardDiskImpl.cpp

@@ -564 +564 @@
 /**
  *  Checks basic accessibility of this hard disk only (w/o parents).
- *  Must be always called by every reimplementation in the first place.
- *
- *  @param aAccessError on output, a null string indicates the hard disk is
+ *  Must be always called by every HardDisk::getAccessible() reimplementation
+ *  in the first place.
+ *
+ *  When @a aCheckBusy is true, this method checks that mBusy = false and
+ *  mReaders = 0 (and returns an appropriate error if not). This lets
+ *  reimplementations successfully call setBusy() to lock the disk for the
+ *  duration of the check if they need. Note that in this case, the
+ *  reimplementation must enter the object lock before calling this method
+ *  and not leave it before calling setBusy() to avoid race condition.
+ *
+ *  @param aAccessError On output, a null string indicates the hard disk is
  *                      accessible, otherwise contains a message describing
  *                      the reason of inaccessibility.
- */
-HRESULT HardDisk::getAccessible (Bstr &aAccessError)
-{
-    AutoLock alock (this);
-    CHECK_READY();
-
-    if (mBusy)
-    {
-        aAccessError =
-            Utf8StrFmt (tr ("Hard disk '%ls' is being used by another task"),
-                        toString().raw());
-    }
-    else
-        aAccessError.setNull();
+ *  @param aCheckBusy   Whether to do the busy check or not.
+ */
+HRESULT HardDisk::getBaseAccessible (Bstr &aAccessError,
+                                     bool aCheckBusy /* = false */)
+{
+    AutoLock alock (this);
+    CHECK_READY();
+
+    aAccessError.setNull();
+
+    if (aCheckBusy)
+    {
+        if (mBusy)
+        {
+            aAccessError = Utf8StrFmt (
+                tr ("Hard disk '%ls' is being exclusively used by another task"),
+                toString().raw());
+        }
+        else if (mReaders > 0)
+        {
+            aAccessError = Utf8StrFmt (
+                tr ("Hard disk '%ls' is being used by another task (%d readers)"),
+                toString().raw(), mReaders);
+        }
+    }
 
     return S_OK;
@@ -1565 +1584 @@
 
     /* check the basic accessibility */
-    HRESULT rc = HardDisk::getAccessible (aAccessError);
+    HRESULT rc = getBaseAccessible (aAccessError, true /* aCheckBusy */);
     if (FAILED (rc) || !aAccessError.isNull())
         return rc;
@@ -3007 +3026 @@
 
     /* check the basic accessibility */
-    HRESULT rc = HardDisk::getAccessible (aAccessError);
+    HRESULT rc = getBaseAccessible (aAccessError);
     if (FAILED (rc) || !aAccessError.isNull())
         return rc;

trunk/src/VBox/Main/include/HardDiskImpl.h

@@ -129 +129 @@
     ComObjPtr <HardDisk> root() const;
 
+    HRESULT getBaseAccessible (Bstr &aAccessError, bool aCheckBusy = false);
+
     // virtual methods that need to be [re]implemented by every subclass
 
     virtual HRESULT trySetRegistered (BOOL aRegistered);
-    virtual HRESULT getAccessible (Bstr &aAccessError);
+    virtual HRESULT getAccessible (Bstr &aAccessError) = 0;
 
     virtual HRESULT saveSettings (CFGNODE aHDNode, CFGNODE aStorageNode) = 0;