Changeset 2845 in vbox for trunk/src/VBox

Timestamp:

May 24, 2007 4:16:41 PM (18 years ago)

Author:

vboxsync

Message:

Main: Switched USBController to the new locking model.

Location:

trunk/src/VBox/Main

Files:

• USBControllerImpl.cpp (modified) (45 diffs)
• include/USBControllerImpl.h (modified) (7 diffs)

trunk/src/VBox/Main/USBControllerImpl.cpp

@@ -43 +43 @@
 /////////////////////////////////////////////////////////////////////////////
 
+DEFINE_EMPTY_CTOR_DTOR (USBController)
+
 HRESULT USBController::FinalConstruct()
 {
@@ -50 +52 @@
 void USBController::FinalRelease()
 {
-    if (isReady())
-        uninit();
+    uninit();
 }
 
@@ -61 +62 @@
  *
  * @returns COM result indicator.
- * @param a_pParent     Pointer to our parent object.
- */
-HRESULT USBController::init(Machine *a_pParent)
-{
-    LogFlowMember(("USBController::init (%p)\n", a_pParent));
-
-    ComAssertRet (a_pParent, E_INVALIDARG);
-
-    AutoLock alock(this);
-    ComAssertRet (!isReady(), E_UNEXPECTED);
-
-    m_Parent = a_pParent;
-    // m_Peer is left null
-
-    m_Data.allocate();
-    m_DeviceFilters.allocate();
-
-    setReady(true);
+ * @param aParent       Pointer to our parent object.
+ */
+HRESULT USBController::init (Machine *aParent)
+{
+    LogFlowThisFunc (("aParent=%p\n", aParent));
+
+    ComAssertRet (aParent, E_INVALIDARG);
+
+    /* Enclose the state transition NotReady->InInit->Ready */
+    AutoInitSpan autoInitSpan (this);
+    AssertReturn (autoInitSpan.isOk(), E_UNEXPECTED);
+
+    unconst (mParent) = aParent;
+    /* mPeer is left null */
+
+    mData.allocate();
+    mDeviceFilters.allocate();
+
+    /* Confirm a successful initialization */
+    autoInitSpan.setSucceeded();
+
     return S_OK;
 }
@@ -89 +93 @@
  *
  * @returns COM result indicator.
- * @param a_pParent     Pointer to our parent object.
- * @param a_pPeer       The object to share.
- *
- * @remark This object must be destroyed before the original object
+ * @param aParent       Pointer to our parent object.
+ * @param aPeer         The object to share.
+ *
+ * @note This object must be destroyed before the original object
  * it shares data with is destroyed.
  */
-HRESULT USBController::init(Machine *a_pParent, USBController *a_pPeer)
-{
-    LogFlowMember(("USBController::init (%p, %p)\n", a_pParent, a_pPeer));
-
-    ComAssertRet (a_pParent && a_pPeer, E_INVALIDARG);
-
-    AutoLock alock(this);
-    ComAssertRet (!isReady(), E_UNEXPECTED);
-
-    m_Parent = a_pParent;
-    m_Peer = a_pPeer;
-
-    AutoLock thatlock(a_pPeer);
-    m_Data.share(a_pPeer->m_Data);
-
-    // create copies of all filters
-    m_DeviceFilters.allocate();
-    DeviceFilterList::const_iterator it = a_pPeer->m_DeviceFilters->begin();
-    while (it != a_pPeer->m_DeviceFilters->end())
+HRESULT USBController::init (Machine *aParent, USBController *aPeer)
+{
+    LogFlowThisFunc (("aParent=%p, aPeer=%p\n", aParent, aPeer));
+
+    ComAssertRet (aParent && aPeer, E_INVALIDARG);
+
+    /* Enclose the state transition NotReady->InInit->Ready */
+    AutoInitSpan autoInitSpan (this);
+    AssertReturn (autoInitSpan.isOk(), E_UNEXPECTED);
+
+    unconst (mParent) = aParent;
+    unconst (mPeer) = aPeer;
+
+    AutoLock thatlock (aPeer);
+    mData.share (aPeer->mData);
+
+    /* create copies of all filters */
+    mDeviceFilters.allocate();
+    DeviceFilterList::const_iterator it = aPeer->mDeviceFilters->begin();
+    while (it != aPeer->mDeviceFilters->end())
     {
         ComObjPtr <USBDeviceFilter> filter;
         filter.createObject();
         filter->init (this, *it);
-        m_DeviceFilters->push_back (filter);
+        mDeviceFilters->push_back (filter);
         ++ it;
     }
 
-    setReady(true);
+    /* Confirm a successful initialization */
+    autoInitSpan.setSucceeded();
+
     return S_OK;
 }
@@ -132 +139 @@
  *  of the original object passed as an argument.
  */
-HRESULT USBController::initCopy (Machine *a_pParent, USBController *a_pPeer)
-{
-    LogFlowMember(("USBController::initCopy (%p, %p)\n", a_pParent, a_pPeer));
-
-    ComAssertRet (a_pParent && a_pPeer, E_INVALIDARG);
-
-    AutoLock alock(this);
-    ComAssertRet (!isReady(), E_UNEXPECTED);
-
-    m_Parent = a_pParent;
-    // m_Peer is left null
-
-    AutoLock thatlock(a_pPeer);
-    m_Data.attachCopy (a_pPeer->m_Data);
-
-    // create private copies of all filters
-    m_DeviceFilters.allocate();
-    DeviceFilterList::const_iterator it = a_pPeer->m_DeviceFilters->begin();
-    while (it != a_pPeer->m_DeviceFilters->end())
+HRESULT USBController::initCopy (Machine *aParent, USBController *aPeer)
+{
+    LogFlowThisFunc (("aParent=%p, aPeer=%p\n", aParent, aPeer));
+
+    ComAssertRet (aParent && aPeer, E_INVALIDARG);
+
+    /* Enclose the state transition NotReady->InInit->Ready */
+    AutoInitSpan autoInitSpan (this);
+    AssertReturn (autoInitSpan.isOk(), E_UNEXPECTED);
+
+    unconst (mParent) = aParent;
+    /* mPeer is left null */
+
+    AutoLock thatlock (aPeer);
+    mData.attachCopy (aPeer->mData);
+
+    /* create private copies of all filters */
+    mDeviceFilters.allocate();
+    DeviceFilterList::const_iterator it = aPeer->mDeviceFilters->begin();
+    while (it != aPeer->mDeviceFilters->end())
     {
         ComObjPtr <USBDeviceFilter> filter;
         filter.createObject();
         filter->initCopy (this, *it);
-        m_DeviceFilters->push_back (filter);
+        mDeviceFilters->push_back (filter);
         ++ it;
     }
 
-    setReady(true);
+    /* Confirm a successful initialization */
+    autoInitSpan.setSucceeded();
+
     return S_OK;
 }
@@ -170 +180 @@
 void USBController::uninit()
 {
-    LogFlowMember(("USBController::uninit()\n"));
-
-    AutoLock alock(this);
-    AssertReturn (isReady(), (void) 0);
-
-    // uninit all filters (including those still referenced by clients)
+    LogFlowThisFunc (("\n"));
+
+    /* Enclose the state transition Ready->InUninit->NotReady */
+    AutoUninitSpan autoUninitSpan (this);
+    if (autoUninitSpan.uninitDone())
+        return;
+
+    /* uninit all filters (including those still referenced by clients) */
     uninitDependentChildren();
 
-    m_DeviceFilters.free();
-    m_Data.free();
-
-    m_Peer.setNull();
-    m_Parent.setNull();
-
-    setReady(false);
-}
-
+    mDeviceFilters.free();
+    mData.free();
+
+    unconst (mPeer).setNull();
+    unconst (mParent).setNull();
+}
 
 
@@ -192 +201 @@
 /////////////////////////////////////////////////////////////////////////////
 
-STDMETHODIMP USBController::COMGETTER(Enabled)(BOOL *a_pfEnabled)
-{
-    if (!a_pfEnabled)
+STDMETHODIMP USBController::COMGETTER(Enabled) (BOOL *aEnabled)
+{
+    if (!aEnabled)
         return E_POINTER;
 
-    AutoLock alock(this);
-    CHECK_READY();
-
-    *a_pfEnabled = m_Data->m_fEnabled;
-    LogFlowMember(("USBController::GetEnabled: returns %p\n", *a_pfEnabled));
-    return S_OK;
-}
-
-
-STDMETHODIMP USBController::COMSETTER(Enabled)(BOOL a_fEnabled)
-{
-    LogFlowMember(("USBController::SetEnabled: %s\n", a_fEnabled ? "enable" : "disable"));
-
-    AutoLock alock(this);
-    CHECK_READY();
-
-    CHECK_MACHINE_MUTABILITY (m_Parent);
-
-    if (m_Data->m_fEnabled != a_fEnabled)
-    {
-        m_Data.backup();
-        m_Data->m_fEnabled = a_fEnabled;
-
-        alock.unlock();
-        m_Parent->onUSBControllerChange();
-    }
-
-    return S_OK;
-}
-
-/**
- * Get the USB standard version.
- *
- * @param   a_pusUSBStandard    Where to store the BCD version word.
- */
-STDMETHODIMP USBController::COMGETTER(USBStandard)(USHORT *a_pusUSBStandard)
-{
-    if (!a_pusUSBStandard)
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
+
+    AutoReaderLock alock (this);
+
+    *aEnabled = mData->mEnabled;
+
+    LogFlowThisFunc (("returns %RTbool\n", *aEnabled));
+
+    return S_OK;
+}
+
+
+STDMETHODIMP USBController::COMSETTER(Enabled) (BOOL aEnabled)
+{
+    LogFlowThisFunc (("aEnabled=%RTbool\n", aEnabled));
+
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
+
+    AutoLock alock (this);
+
+    CHECK_MACHINE_MUTABILITY (mParent);
+
+    if (mData->mEnabled != aEnabled)
+    {
+        mData.backup();
+        mData->mEnabled = aEnabled;
+
+        /* leave the lock for safety */
+        alock.leave();
+
+        mParent->onUSBControllerChange();
+    }
+
+    return S_OK;
+}
+
+STDMETHODIMP USBController::COMGETTER(USBStandard) (USHORT *aUSBStandard)
+{
+    if (!aUSBStandard)
         return E_POINTER;
 
-    AutoLock alock(this);
-    CHECK_READY();
-
-    *a_pusUSBStandard = 0x0101;
-    LogFlowMember(("USBController::GetUSBStandard: returns 0x0101\n"));
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
+
+    /* not accessing data -- no need to lock */
+
+    *aUSBStandard = 0x0101;
+
+    LogFlowThisFunc (("returns %04hX\n", *aUSBStandard));
+
     return S_OK;
 }
@@ -250 +266 @@
         return E_POINTER;
 
-    AutoLock lock(this);
-    CHECK_READY();
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
+
+    AutoReaderLock alock (this);
 
     ComObjPtr <USBDeviceFilterCollection> collection;
     collection.createObject();
-    collection->init (*m_DeviceFilters.data());
+    collection->init (*mDeviceFilters.data());
     collection.queryInterfaceTo (aDevicesFilters);
+
     return S_OK;
 }
@@ -272 +291 @@
         return E_INVALIDARG;
 
-    AutoLock lock (this);
-    CHECK_READY();
-
-    CHECK_MACHINE_MUTABILITY (m_Parent);
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
+
+    AutoLock alock (this);
+
+    CHECK_MACHINE_MUTABILITY (mParent);
 
     ComObjPtr <USBDeviceFilter> filter;
     filter.createObject();
     HRESULT rc = filter->init (this, aName);
-    ComAssertComRCRet (rc, rc);
+    ComAssertComRCRetRC (rc);
     rc = filter.queryInterfaceTo (aFilter);
-    AssertComRCReturn (rc, rc);
+    AssertComRCReturnRC (rc);
+
     return S_OK;
 }
@@ -292 +314 @@
         return E_INVALIDARG;
 
-    AutoLock lock (this);
-    CHECK_READY();
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
 
     /* the machine needs to be mutable */
-    Machine::AutoStateDependency <Machine::MutableStateDep> adep (m_Parent);
+    Machine::AutoStateDependency <Machine::MutableStateDep> adep (mParent);
     CheckComRCReturnRC (adep.rc());
+
+    AutoLock alock (this);
 
     ComObjPtr <USBDeviceFilter> filter = getDependentChild (aFilter);
@@ -310 +334 @@
 
     /* backup the list before modification */
-    m_DeviceFilters.backup();
+    mDeviceFilters.backup();
 
     /* iterate to the position... */
     DeviceFilterList::iterator it;
-    if (aPosition < m_DeviceFilters->size())
-    {
-        it = m_DeviceFilters->begin();
+    if (aPosition < mDeviceFilters->size())
+    {
+        it = mDeviceFilters->begin();
         std::advance (it, aPosition);
     }
     else
-        it = m_DeviceFilters->end();
+        it = mDeviceFilters->end();
     /* ...and insert */
-    m_DeviceFilters->insert (it, filter);
+    mDeviceFilters->insert (it, filter);
     filter->mInList = true;
 
@@ -335 +359 @@
 #endif
     {
-        USBProxyService *service = m_Parent->virtualBox()->host()->usbProxyService();
+        USBProxyService *service = mParent->virtualBox()->host()->usbProxyService();
         ComAssertRet (service, E_FAIL);
 
@@ -351 +375 @@
         return E_POINTER;
 
-    AutoLock lock (this);
-    CHECK_READY();
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
 
     /* the machine needs to be mutable */
-    Machine::AutoStateDependency <Machine::MutableStateDep> adep (m_Parent);
+    Machine::AutoStateDependency <Machine::MutableStateDep> adep (mParent);
     CheckComRCReturnRC (adep.rc());
 
-    if (!m_DeviceFilters->size())
+    AutoLock alock (this);
+
+    if (!mDeviceFilters->size())
         return setError (E_INVALIDARG,
             tr ("The USB device filter list is empty"));
 
-    if (aPosition >= m_DeviceFilters->size())
+    if (aPosition >= mDeviceFilters->size())
         return setError (E_INVALIDARG,
             tr ("Invalid position: %lu (must be in range [0, %lu])"),
-            aPosition, m_DeviceFilters->size() - 1);
+            aPosition, mDeviceFilters->size() - 1);
 
     /* backup the list before modification */
-    m_DeviceFilters.backup();
+    mDeviceFilters.backup();
 
     ComObjPtr <USBDeviceFilter> filter;
     {
         /* iterate to the position... */
-        DeviceFilterList::iterator it = m_DeviceFilters->begin();
+        DeviceFilterList::iterator it = mDeviceFilters->begin();
         std::advance (it, aPosition);
         /* ...get an element from there... */
@@ -379 +405 @@
         /* ...and remove */
         filter->mInList = false;
-        m_DeviceFilters->erase (it);
+        mDeviceFilters->erase (it);
     }
 
@@ -397 +423 @@
 #endif
     {
-        USBProxyService *service = m_Parent->virtualBox()->host()->usbProxyService();
+        USBProxyService *service = mParent->virtualBox()->host()->usbProxyService();
         ComAssertRet (service, E_FAIL);
 
@@ -411 +437 @@
 /////////////////////////////////////////////////////////////////////////////
 
-/** Loads settings from the configuration node */
+/**
+ *  Loads settings from the configuration node.
+ *
+ *  @note Locks objects for reading!
+ */
 HRESULT USBController::loadSettings (CFGNODE aMachine)
 {
-    AutoLock lock (this);
-    CHECK_READY();
-
     ComAssertRet (aMachine, E_FAIL);
+
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
+
+    AutoLock alock (this);
 
     CFGNODE controller = NULL;
@@ -423 +455 @@
     Assert (controller);
 
-    // enabled
+    /* enabled */
     bool enabled;
     CFGLDRQueryBool (controller, "enabled", &enabled);
-    m_Data->m_fEnabled = enabled;
+    mData->mEnabled = enabled;
 
     HRESULT rc = S_OK;
@@ -466 +498 @@
                               manufacturer, product, serialNumber,
                               port, remote);
-        // error info is set by init() when appropriate
+        /* error info is set by init() when appropriate */
         if (SUCCEEDED (rc))
         {
-            m_DeviceFilters->push_back (filterObj);
+            mDeviceFilters->push_back (filterObj);
             filterObj->mInList = true;
         }
@@ -481 +513 @@
 }
 
-/** Saves settings to the configuration node */
+/**
+ *  Saves settings to the configuration node.
+ *
+ *  @note Locks objects for reading!
+ */
 HRESULT USBController::saveSettings (CFGNODE aMachine)
 {
-    AutoLock lock (this);
-    CHECK_READY();
-
     ComAssertRet (aMachine, E_FAIL);
 
-    // first, delete the entry
+    AutoCaller autoCaller (this);
+    CheckComRCReturnRC (autoCaller.rc());
+
+    AutoReaderLock alock (this);
+
+    /* first, delete the entry */
     CFGNODE controller = NULL;
     int vrc = CFGLDRGetChildNode (aMachine, "USBController", 0, &controller);
@@ -497 +535 @@
         ComAssertRCRet (vrc, E_FAIL);
     }
-    // then, recreate it
+    /* then, recreate it */
     vrc = CFGLDRCreateChildNode (aMachine, "USBController", &controller);
     ComAssertRCRet (vrc, E_FAIL);
 
-    // enabled
-    CFGLDRSetBool (controller, "enabled", !!m_Data->m_fEnabled);
-
-    DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-    while (it != m_DeviceFilters->end())
+    /* enabled */
+    CFGLDRSetBool (controller, "enabled", !!mData->mEnabled);
+
+    DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+    while (it != mDeviceFilters->end())
     {
         AutoLock filterLock (*it);
@@ -516 +554 @@
         CFGLDRSetBool (filter, "active", !!data.mActive);
 
-        // all are optional
+        /* all are optional */
         if (data.mVendorId.string())
             CFGLDRSetBSTR (filter, "vendorid", data.mVendorId.string());
@@ -544 +582 @@
 }
 
+/** @note Locks objects for reading! */
 bool USBController::isModified()
 {
-    AutoLock alock(this);
-
-    if (m_Data.isBackedUp() || m_DeviceFilters.isBackedUp())
+    AutoCaller autoCaller (this);
+    AssertComRCReturn (autoCaller.rc(), false);
+
+    AutoReaderLock alock (this);
+
+    if (mData.isBackedUp() || mDeviceFilters.isBackedUp())
         return true;
 
     /* see whether any of filters has changed its data */
     for (DeviceFilterList::const_iterator
-         it = m_DeviceFilters->begin();
-         it != m_DeviceFilters->end();
+         it = mDeviceFilters->begin();
+         it != mDeviceFilters->end();
          ++ it)
     {
@@ -564 +606 @@
 }
 
+/** @note Locks objects for reading! */
 bool USBController::isReallyModified()
 {
-    AutoLock alock(this);
-
-    if (m_Data.hasActualChanges())
+    AutoCaller autoCaller (this);
+    AssertComRCReturn (autoCaller.rc(), false);
+
+    AutoReaderLock alock (this);
+
+    if (mData.hasActualChanges())
         return true;
 
-    if (!m_DeviceFilters.isBackedUp())
+    if (!mDeviceFilters.isBackedUp())
     {
         /* see whether any of filters has changed its data */
         for (DeviceFilterList::const_iterator
-             it = m_DeviceFilters->begin();
-             it != m_DeviceFilters->end();
+             it = mDeviceFilters->begin();
+             it != mDeviceFilters->end();
              ++ it)
         {
@@ -586 +632 @@
     }
 
-    if (m_DeviceFilters->size() != m_DeviceFilters.backedUpData()->size())
+    if (mDeviceFilters->size() != mDeviceFilters.backedUpData()->size())
         return true;
 
-    if (m_DeviceFilters->size() == 0)
+    if (mDeviceFilters->size() == 0)
         return false;
 
     /* Make copies to speed up comparison */
-    DeviceFilterList devices = *m_DeviceFilters.data();
-    DeviceFilterList backDevices = *m_DeviceFilters.backedUpData();
+    DeviceFilterList devices = *mDeviceFilters.data();
+    DeviceFilterList backDevices = *mDeviceFilters.backedUpData();
 
     DeviceFilterList::iterator it = devices.begin();
@@ -623 +669 @@
 }
 
+/** @note Locks objects for writing! */
 bool USBController::rollback()
 {
+    AutoCaller autoCaller (this);
+    AssertComRCReturn (autoCaller.rc(), false);
+
+    /* we need the machine state */
+    Machine::AutoStateDependency <Machine::MutableStateDep> adep (mParent);
+    AssertComRCReturn (adep.rc(), false);
+
     AutoLock alock (this);
 
-    /* we need the machine state */
-    Machine::AutoStateDependency <Machine::MutableStateDep> adep (m_Parent);
-    AssertComRCReturn (adep.rc(), false);
-
     bool dataChanged = false;
 
-    if (m_Data.isBackedUp())
+    if (mData.isBackedUp())
     {
         /* we need to check all data to see whether anything will be changed
          * after rollback */
-        dataChanged = m_Data.hasActualChanges();
-        m_Data.rollback();
-    }
-
-    if (m_DeviceFilters.isBackedUp())
-    {
-        USBProxyService *service = m_Parent->virtualBox()->host()->usbProxyService();
+        dataChanged = mData.hasActualChanges();
+        mData.rollback();
+    }
+
+    if (mDeviceFilters.isBackedUp())
+    {
+        USBProxyService *service = mParent->virtualBox()->host()->usbProxyService();
         ComAssertRet (service, false);
 
         /* uninitialize all new filters (absent in the backed up list) */
-        DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-        DeviceFilterList *backedList = m_DeviceFilters.backedUpData();
-        while (it != m_DeviceFilters->end())
+        DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+        DeviceFilterList *backedList = mDeviceFilters.backedUpData();
+        while (it != mDeviceFilters->end())
         {
             if (std::find (backedList->begin(), backedList->end(), *it) ==
@@ -688 +738 @@
             while (it != backedList->end())
             {
-                if (std::find (m_DeviceFilters->begin(), m_DeviceFilters->end(), *it) ==
-                    m_DeviceFilters->end())
+                if (std::find (mDeviceFilters->begin(), mDeviceFilters->end(), *it) ==
+                    mDeviceFilters->end())
                 {
                     /* notify the proxy (only when necessary) */
@@ -705 +755 @@
 
         /* restore the list */
-        m_DeviceFilters.rollback();
+        mDeviceFilters.rollback();
     }
 
@@ -712 +762 @@
 
     /* rollback any changes to filters after restoring the list */
-    DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-    while (it != m_DeviceFilters->end())
+    DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+    while (it != mDeviceFilters->end())
     {
         if ((*it)->isModified())
@@ -727 +777 @@
 }
 
+/** @note Locks objects for writing! */
 void USBController::commit()
 {
+    AutoCaller autoCaller (this);
+    AssertComRCReturnVoid (autoCaller.rc());
+
     AutoLock alock (this);
 
-    if (m_Data.isBackedUp())
-    {
-        m_Data.commit();
-        if (m_Peer)
+    if (mData.isBackedUp())
+    {
+        mData.commit();
+        if (mPeer)
         {
             // attach new data to the peer and reshare it
-            AutoLock peerlock (m_Peer);
-            m_Peer->m_Data.attach (m_Data);
+            AutoLock peerlock (mPeer);
+            mPeer->mData.attach (mData);
         }
     }
@@ -744 +798 @@
     bool commitFilters = false;
 
-    if (m_DeviceFilters.isBackedUp())
-    {
-        m_DeviceFilters.commit();
+    if (mDeviceFilters.isBackedUp())
+    {
+        mDeviceFilters.commit();
 
         // apply changes to peer
-        if (m_Peer)
-        {
-            AutoLock peerlock (m_Peer);
+        if (mPeer)
+        {
+            AutoLock peerlock (mPeer);
             // commit all changes to new filters (this will reshare data with
             // peers for those who have peers)
             DeviceFilterList *newList = new DeviceFilterList();
-            DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-            while (it != m_DeviceFilters->end())
+            DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+            while (it != mDeviceFilters->end())
             {
                 (*it)->commit();
@@ -767 +821 @@
                     // create a peer owning data this filter share it with
                     peer.createObject();
-                    peer->init (m_Peer, *it, true /* aReshare */);
+                    peer->init (mPeer, *it, true /* aReshare */);
                 }
                 else
                 {
                     // remove peer from the old list
-                    m_Peer->m_DeviceFilters->remove (peer);
+                    mPeer->mDeviceFilters->remove (peer);
                 }
                 // and add it to the new list
@@ -781 +835 @@
 
             // uninit old peer's filters that are left
-            it = m_Peer->m_DeviceFilters->begin();
-            while (it != m_Peer->m_DeviceFilters->end())
+            it = mPeer->mDeviceFilters->begin();
+            while (it != mPeer->mDeviceFilters->end())
             {
                 (*it)->uninit();
@@ -789 +843 @@
 
             // attach new list of filters to our peer
-            m_Peer->m_DeviceFilters.attach (newList);
+            mPeer->mDeviceFilters.attach (newList);
         }
         else
@@ -807 +861 @@
     if (commitFilters)
     {
-        DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-        while (it != m_DeviceFilters->end())
+        DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+        while (it != mDeviceFilters->end())
         {
             (*it)->commit();
@@ -816 +870 @@
 }
 
+/** @note Locks object for writing and that object for reading! */
 void USBController::copyFrom (USBController *aThat)
 {
-    AutoLock alock (this);
-    AutoLock thatLock (aThat);
-
-    if (m_Parent->isRegistered())
-    {
-        // reuse onMachineRegistered to tell USB proxy to remove all current filters
+    AutoCaller autoCaller (this);
+    AssertComRCReturnVoid (autoCaller.rc());
+
+    AutoMultiLock <2> alock (this->wlock(), aThat->rlock());
+
+    if (mParent->isRegistered())
+    {
+        /* reuse onMachineRegistered to tell USB proxy to remove all current
+           filters */
         HRESULT rc = onMachineRegistered (FALSE);
         AssertComRCReturn (rc, (void) 0);
     }
 
-    // this will back up current data
-    m_Data.assignCopy (aThat->m_Data);
-
-    // create private copies of all filters
-    m_DeviceFilters.backup();
-    m_DeviceFilters->clear();
-    for (DeviceFilterList::const_iterator it = aThat->m_DeviceFilters->begin();
-        it != aThat->m_DeviceFilters->end();
+    /* this will back up current data */
+    mData.assignCopy (aThat->mData);
+
+    /* create private copies of all filters */
+    mDeviceFilters.backup();
+    mDeviceFilters->clear();
+    for (DeviceFilterList::const_iterator it = aThat->mDeviceFilters->begin();
+        it != aThat->mDeviceFilters->end();
         ++ it)
     {
@@ -841 +899 @@
         filter.createObject();
         filter->initCopy (this, *it);
-        m_DeviceFilters->push_back (filter);
-    }
-
-    if (m_Parent->isRegistered())
-    {
-        // reuse onMachineRegistered to tell USB proxy to insert all current filters
+        mDeviceFilters->push_back (filter);
+    }
+
+    if (mParent->isRegistered())
+    {
+        /* reuse onMachineRegistered to tell USB proxy to insert all current
+           filters */
         HRESULT rc = onMachineRegistered (TRUE);
         AssertComRCReturn (rc, (void) 0);
@@ -857 +916 @@
  *
  *  @param aRegistered  new registered state of the machine
+ *
+ *  @note Locks nothing.
  */
 HRESULT USBController::onMachineRegistered (BOOL aRegistered)
 {
-    AutoLock alock (this);
-    CHECK_READY();
+    AutoCaller autoCaller (this);
+    AssertComRCReturnRC (autoCaller.rc());
 
     /// @todo After rewriting Win32 USB support, no more necessary;
@@ -874 +935 @@
 /**
  *  Called by setter methods of all USB device filters.
+ *
+ *  @note Locks nothing.
  */
 HRESULT USBController::onDeviceFilterChange (USBDeviceFilter *aFilter,
                                              BOOL aActiveChanged /* = FALSE */)
 {
-    AutoLock alock (this);
-    CHECK_READY();
+    AutoCaller autoCaller (this);
+    AssertComRCReturnRC (autoCaller.rc());
 
     /// @todo After rewriting Win32 USB support, no more necessary;
@@ -886 +949 @@
 #else
     /* we need the machine state */
-    Machine::AutoStateDependency <Machine::MutableStateDep> adep (m_Parent);
+    Machine::AutoStateDependency <Machine::MutableStateDep> adep (mParent);
     AssertComRCReturnRC (adep.rc());
 
@@ -894 +957 @@
 #endif
 
-    if (aFilter->mInList && m_Parent->isRegistered())
-    {
-        USBProxyService *service = m_Parent->virtualBox()->host()->usbProxyService();
+    /* we don't modify our data fields -- no need to lock */
+
+    if (aFilter->mInList && mParent->isRegistered())
+    {
+        USBProxyService *service = mParent->virtualBox()->host()->usbProxyService();
         ComAssertRet (service, E_FAIL);
 
@@ -936 +1001 @@
  *
  *  A HostUSBDevice specific version.
+ *
+ *  @note Locks this object for reading.
  */
 bool USBController::hasMatchingFilter (ComObjPtr <HostUSBDevice> &aDevice)
 {
-    AutoLock alock (this);
-    if (!isReady())
-        return false;
+    AutoCaller autoCaller (this);
+    AssertComRCReturn (autoCaller.rc(), false);
+
+    AutoReaderLock alock (this);
 
     bool match = false;
 
-    // apply self filters
-    for (DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-         !match && it != m_DeviceFilters->end();
+    /* apply self filters */
+    for (DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+         !match && it != mDeviceFilters->end();
          ++ it)
     {
@@ -966 +1034 @@
  *      This method MUST correlate with HostUSBDevice::isMatch()
  *      in the sense of the device matching logic.
+ *
+ *  @note Locks this object for reading.
  */
 bool USBController::hasMatchingFilter (IUSBDevice *aUSBDevice)
 {
-    LogFlowThisFunc (("\n"));
-
-    AutoLock alock (this);
-    if (!isReady())
-        return false;
+    LogFlowThisFuncEnter();
+
+    AutoCaller autoCaller (this);
+    AssertComRCReturn (autoCaller.rc(), false);
+
+    AutoReaderLock alock (this);
 
     HRESULT rc = S_OK;
 
-    // query fields
+    /* query fields */
 
     USHORT vendorId = 0;
@@ -1020 +1091 @@
     bool match = false;
 
-    // apply self filters
-    for (DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-         it != m_DeviceFilters->end();
+    /* apply self filters */
+    for (DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+         it != mDeviceFilters->end();
          ++ it)
     {
@@ -1038 +1109 @@
 
 #if !defined (__WIN__)
-        // these filters are temporarily ignored on Win32
+        /* these filters are temporarily ignored on Win32 */
         if (!aData.mManufacturer.isMatch (manufacturer))
             continue;
@@ -1056 +1127 @@
     }
 
-    LogFlowMember (("USBController::hasMatchingFilter() returns: %d\n", match));
+    LogFlowThisFunc (("returns: %d\n", match));
+    LogFlowThisFuncLeave();
+
     return match;
 }
 
+/**
+ *  Notifies the proxy service about all filters as requested by the
+ *  @a aInsertFilters argument.
+ *
+ *  @param aInsertFilters   @c true to insert filters, @c false to remove.
+ *
+ *  @note Locks this object for reading.
+ */
 HRESULT USBController::notifyProxy (bool aInsertFilters)
 {
     LogFlowThisFunc (("aInsertFilters=%RTbool\n", aInsertFilters));
 
-    AutoLock alock (this);
-    if (!isReady())
-        return false;
-
-    USBProxyService *service = m_Parent->virtualBox()->host()->usbProxyService();
+    AutoCaller autoCaller (this);
+    AssertComRCReturn (autoCaller.rc(), false);
+
+    AutoReaderLock alock (this);
+
+    USBProxyService *service = mParent->virtualBox()->host()->usbProxyService();
     AssertReturn (service, E_FAIL);
 
-    DeviceFilterList::const_iterator it = m_DeviceFilters->begin();
-    while (it != m_DeviceFilters->end())
+    DeviceFilterList::const_iterator it = mDeviceFilters->begin();
+    while (it != mDeviceFilters->end())
     {
         USBDeviceFilter *flt = *it; /* resolve ambiguity (for ComPtr below) */

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

@@ -33 +33 @@
 class HostUSBDevice;
 
+/**
+ *  @note we cannot use VirtualBoxBaseWithTypedChildren <USBDeviceFilter> as a
+ *  base class, because we want a quick (map-based) way of validating
+ *  IUSBDeviceFilter pointers passed from outside as method parameters that
+ *  VirtualBoxBaseWithChildren::getDependentChild() gives us.
+ */
+
 class ATL_NO_VTABLE USBController :
-    public VirtualBoxBaseWithChildren,
-    public VirtualBoxSupportErrorInfoImpl<USBController, IUSBController>,
-    public VirtualBoxSupportTranslation<USBController>,
+    public VirtualBoxBaseWithChildrenNEXT,
+    public VirtualBoxSupportErrorInfoImpl <USBController, IUSBController>,
+    public VirtualBoxSupportTranslation <USBController>,
     public IUSBController
 {
@@ -44 +51 @@
     {
         /* Constructor. */
-        Data() : m_fEnabled(FALSE) { }
+        Data() : mEnabled (FALSE) { }
 
         bool operator== (const Data &that) const
         {
-            return this == &that || m_fEnabled == that.m_fEnabled;
+            return this == &that || mEnabled == that.mEnabled;
         }
 
         /** Enabled indicator. */
-        BOOL                            m_fEnabled;
+        BOOL mEnabled;
     };
 
 public:
 
-    DECLARE_NOT_AGGREGATABLE(USBController)
+    VIRTUALBOXBASE_ADD_ERRORINFO_SUPPORT (USBController)
+
+    DECLARE_NOT_AGGREGATABLE (USBController)
 
     DECLARE_PROTECT_FINAL_CONSTRUCT()
 
     BEGIN_COM_MAP(USBController)
-        COM_INTERFACE_ENTRY(ISupportErrorInfo)
-        COM_INTERFACE_ENTRY(IUSBController)
+        COM_INTERFACE_ENTRY (ISupportErrorInfo)
+        COM_INTERFACE_ENTRY (IUSBController)
     END_COM_MAP()
 
     NS_DECL_ISUPPORTS
+
+    DECLARE_EMPTY_CTOR_DTOR (USBController)
 
     HRESULT FinalConstruct();
@@ -72 +83 @@
 
     // public initializer/uninitializer for internal purposes only
-    HRESULT init (Machine *parent);
-    HRESULT init (Machine *parent, USBController *that);
-    HRESULT initCopy (Machine *parent, USBController *that);
+    HRESULT init (Machine *aParent);
+    HRESULT init (Machine *aParent, USBController *aThat);
+    HRESULT initCopy (Machine *aParent, USBController *aThat);
     void uninit();
 
     // IUSBController properties
-    STDMETHOD(COMGETTER(Enabled))(BOOL *a_pfEnabled);
-    STDMETHOD(COMSETTER(Enabled))(BOOL a_fEnabled);
-    STDMETHOD(COMGETTER(USBStandard))(USHORT *a_pusUSBStandard);
-    STDMETHOD(COMGETTER(DeviceFilters))(IUSBDeviceFilterCollection **aDevicesFilters);
+    STDMETHOD(COMGETTER(Enabled)) (BOOL *aEnabled);
+    STDMETHOD(COMSETTER(Enabled)) (BOOL aEnabled);
+    STDMETHOD(COMGETTER(USBStandard)) (USHORT *aUSBStandard);
+    STDMETHOD(COMGETTER(DeviceFilters)) (IUSBDeviceFilterCollection **aDevicesFilters);
 
     // IUSBController methods
@@ -90 +101 @@
     // public methods only for internal purposes
 
-    ComObjPtr <Machine, ComWeakRef> &parent() { return m_Parent; };
+    const ComObjPtr <Machine, ComWeakRef> &parent() { return mParent; };
 
     HRESULT loadSettings (CFGNODE aMachine);
@@ -101 +112 @@
     void copyFrom (USBController *aThat);
 
-    const Backupable<Data> &data() { return m_Data; }
+    const Backupable<Data> &data() { return mData; }
 
     HRESULT onMachineRegistered (BOOL aRegistered);
@@ -130 +141 @@
 
     /** Parent object. */
-    ComObjPtr<Machine, ComWeakRef>  m_Parent;
+    const ComObjPtr<Machine, ComWeakRef> mParent;
     /** Peer object. */
-    ComObjPtr<USBController>        m_Peer;
+    const ComObjPtr <USBController> mPeer;
     /** Data. */
-    Backupable<Data>                m_Data;
+    Backupable <Data> mData;
 
     // the following fields need special backup/rollback/commit handling,
@@ -140 +151 @@
 
     typedef std::list <ComObjPtr <USBDeviceFilter> > DeviceFilterList;
-    Backupable <DeviceFilterList> m_DeviceFilters;
+    Backupable <DeviceFilterList> mDeviceFilters;
 };