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

Timestamp:

Mar 14, 2012 2:28:33 PM (13 years ago)

Author:

vboxsync

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

76831

Message:

Main/Console: fix various regression from the locking cleanup

File:

• trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (modified) (33 diffs)

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

@@ -2435 +2435 @@
         return ptrVM.rc();
 
-/** @todo release the console lock? */
+    // no need to release lock, as there are no cross-thread callbacks
 
     /* get the acpi device interface and press the button. */
@@ -2483 +2483 @@
         return ptrVM.rc();
 
-/** @todo release the console lock? */
+    // no need to release lock, as there are no cross-thread callbacks
 
     /* get the acpi device interface and check if the button press was handled. */
@@ -2538 +2538 @@
         return ptrVM.rc();
 
-/** @todo release the console lock? */
+    // no need to release lock, as there are no cross-thread callbacks
 
     /* get the acpi device interface and query the information. */
@@ -2579 +2579 @@
         return ptrVM.rc();
 
-/** @todo release the console lock? */
+    // no need to release lock, as there are no cross-thread callbacks
 
     /* get the acpi device interface and press the sleep button. */
@@ -2800 +2800 @@
 }
 
-/** read the value of a LEd. */
+/** read the value of a LED. */
 inline uint32_t readAndClearLed(PPDMLED pLed)
 {
@@ -4323 +4323 @@
                 AssertComRC(rc);
                 const char *pszAdapterName = networkAdapterTypeToName(adapterType);
+
+                // prevent cross-thread deadlocks, don't need the lock any more
+                alock.release();
+
                 PPDMIBASE pBase;
                 int vrc = PDMR3QueryDeviceLun(ptrVM, pszAdapterName, ulInstance, 0, &pBase);
@@ -4374 +4378 @@
     }
 
+    // definitely don't need the lock any more
+    alock.release();
+
     /* notify console callbacks on success */
     if (SUCCEEDED(rc))
-    {
-        alock.release(); /** @todo 101% safe? */
         fireNetworkAdapterChangedEvent(mEventSource, aNetworkAdapter);
-    }
 
     LogFlowThisFunc(("Leaving rc=%#x\n", rc));
@@ -4485 +4489 @@
  * @param   uLun                The PDM LUN number of the drive.
  * @param   aNetworkAdapter     The network adapter whose attachment needs to be changed
- *
- * @note Locks this object for writing.
  */
 HRESULT Console::doNetworkAdapterChange(PVM pVM,
@@ -4499 +4501 @@
     AutoCaller autoCaller(this);
     AssertComRCReturnRC(autoCaller.rc());
-
-    /* We will need to release the write lock before calling EMT */
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     /* Get the VM handle. */
@@ -4517 +4516 @@
                           (PFNRT) Console::changeNetworkAttachment, 6,
                           this, ptrVM.raw(), pszDevice, uInstance, uLun, aNetworkAdapter);
-
-    /* release the lock before waiting for a result (EMT will call us back!) */
-    alock.release();
 
     if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc))
@@ -4968 +4964 @@
     if (aError != NULL)
     {
+        alock.release();
         /* notify callbacks about the error */
         onUSBDeviceStateChange(aDevice, true /* aAttached */, aError);
@@ -4979 +4976 @@
         return E_FAIL;
     }
+
+    alock.release();
 
     HRESULT rc = attachUSBDevice(aDevice, aMaskedIfs);
@@ -5055 +5054 @@
         AssertFailedReturn(E_FAIL);
     }
+
+    alock.release();
 
     if (aError != NULL)
@@ -5154 +5155 @@
     AssertComRCReturnRC(autoCaller.rc());
 
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     HRESULT rc = S_OK;
 
@@ -5171 +5170 @@
     /* notify console callbacks on success */
     if (SUCCEEDED(rc))
-    {
-        alock.release(); /** @todo 101% safe? */
         fireStorageDeviceChangedEvent(mEventSource, aMediumAttachment, aRemove);
-    }
 
     LogFlowThisFunc(("Leaving rc=%#x\n", rc));
@@ -6813 +6809 @@
             {
                 fHasUSBController = true;
+                alock.release();
                 detachAllUSBDevices(false /* aDone */);
+                alock.acquire();
             }
         }
@@ -6866 +6864 @@
         /* Complete the detaching of the USB devices. */
         if (fHasUSBController)
+        {
+            alock.release();
             detachAllUSBDevices(true /* aDone */);
+            alock.acquire();
+        }
 
         /* advance percent count */
@@ -7661 +7663 @@
     AssertReturn(isWriteLockOnCurrentThread(), E_FAIL);
 
-    /* still want a lock object because we need to release it */
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     HRESULT hrc;
 
@@ -7693 +7692 @@
                       Address.c_str(), uuid.raw()));
 
-    /* release the lock before a VMR3* call (EMT will call us back)! */
-    alock.release();
-
 /** @todo just do everything here and only wrap the PDMR3Usb call. That'll offload some notification stuff from the EMT thread. */
     int vrc = VMR3ReqCallWait(ptrVM, VMCPUID_ANY,
                               (PFNRT)usbAttachCallback, 7,
                               this, ptrVM.raw(), aHostDevice, uuid.raw(), fRemote, Address.c_str(), aMaskedIfs);
-
-    /* restore the lock */
-    alock.acquire();
 
     /* hrc is S_OK here */
@@ -7780 +7773 @@
         LogFlowFunc(("Attached device {%RTuuid}\n", pUSBDevice->id().raw()));
 
+        alock.release();
+
         /* notify callbacks */
         that->onUSBDeviceStateChange(pUSBDevice, true /* aAttached */, NULL);
@@ -7803 +7798 @@
     AssertReturn(isWriteLockOnCurrentThread(), E_FAIL);
 
-    /* still want a lock object because we need to release it */
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
-
     /* Get the VM handle. */
     SafeVMPtr ptrVM(this);
@@ -7816 +7808 @@
     LogFlowThisFunc(("Detaching USB proxy device {%RTuuid}...\n",
                      (*aIt)->id().raw()));
-
-    /* release the lock before a VMR3* call (EMT will call us back)! */
-    alock.release();
 
 /** @todo just do everything here and only wrap the PDMR3Usb call. That'll offload some notification stuff from the EMT thread. */
@@ -7873 +7862 @@
         that->mUSBDevices.erase(*aIt);
         LogFlowFunc(("Detached device {%RTuuid}\n", pUSBDevice->id().raw()));
+
+        alock.release();
 
         /* notify callbacks */
@@ -8263 +8254 @@
  *
  * @param   pVM     The VM handle.
- *
- * @note The caller must lock this object for writing.
  */
 HRESULT Console::captureUSBDevices(PVM pVM)
@@ -8271 +8260 @@
 
     /* sanity check */
-    ComAssertRet(isWriteLockOnCurrentThread(), E_FAIL);
+    AssertReturn(!isWriteLockOnCurrentThread(), E_FAIL);
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     /* If the machine has an USB controller, ask the USB proxy service to
@@ -8282 +8272 @@
          * us back from under its lock (e.g. onUSBDeviceAttach()) which would
          * produce an inter-process dead-lock otherwise. */
-        AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
         alock.release();
 
@@ -8301 +8290 @@
  * Detach all USB device which are attached to the VM for the
  * purpose of clean up and such like.
- *
- * @note The caller must lock this object for writing.
  */
 void Console::detachAllUSBDevices(bool aDone)
@@ -8309 +8296 @@
 
     /* sanity check */
-    AssertReturnVoid(isWriteLockOnCurrentThread());
+    AssertReturnVoid(!isWriteLockOnCurrentThread());
+    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
 
     mUSBDevices.clear();
@@ -8316 +8304 @@
      * us back from under its lock (e.g. onUSBDeviceAttach()) which would
      * produce an inter-process dead-lock otherwise. */
-    AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS);
     alock.release();
 
@@ -8415 +8402 @@
             if (fMatched)
             {
+                alock.release();
                 hrc = onUSBDeviceAttach(pUSBDevice, NULL, fMaskedIfs);
+                alock.acquire();
 
                 /// @todo (r=dmik) warning reporting subsystem
@@ -8480 +8469 @@
             Bstr uuid;
             pUSBDevice->COMGETTER(Id)(uuid.asOutParam());
+            alock.release();
             onUSBDeviceDetach(uuid.raw(), NULL);
+            alock.acquire();
         }
 
@@ -8715 +8706 @@
                 }
 
+                /* release the lock before a lengthy operation */
+                alock.release();
+
                 /*
                  * Capture USB devices.
@@ -8720 +8714 @@
                 rc = pConsole->captureUSBDevices(pVM);
                 if (FAILED(rc)) break;
-
-                /* release the lock before a lengthy operation */
-                alock.release();
 
                 /* Load saved state? */