Index: /trunk/src/VBox/Main/include/ConsoleImpl.h =================================================================== --- /trunk/src/VBox/Main/include/ConsoleImpl.h (revision 39719) +++ /trunk/src/VBox/Main/include/ConsoleImpl.h (revision 39720) @@ -694,4 +694,7 @@ const ComObjPtr mDebugger; const ComObjPtr mVRDEServerInfo; + /** This can safely be used without holding any locks. + * An AutoCaller suffices to prevent it being destroy while in use and + * internally there is a lock providing the necessary serialization. */ const ComObjPtr mEventSource; #ifdef VBOX_WITH_EXTPACK Index: /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 39719) +++ /trunk/src/VBox/Main/src-client/ConsoleImpl.cpp (revision 39720) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2006-2011 Oracle Corporation + * Copyright (C) 2006-2012 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -1853,10 +1853,12 @@ AutoCaller autoCaller(this); - if (FAILED(autoCaller.rc())) return autoCaller.rc(); - - // no need to lock - lifetime constant - mEventSource.queryInterfaceTo(aEventSource); - - return S_OK; + HRESULT hrc = autoCaller.rc(); + if (SUCCEEDED(hrc)) + { + // no need to lock - lifetime constant + mEventSource.queryInterfaceTo(aEventSource); + } + + return hrc; } @@ -3100,5 +3102,6 @@ m_mapSharedFolders.insert(std::make_pair(aName, pSharedFolder)); - /* notify console callbacks after the folder is added to the list */ + /* Notify console callbacks after the folder is added to the list. */ + alock.release(); fireSharedFolderChangedEvent(mEventSource, Scope_Session); @@ -3165,5 +3168,6 @@ m_mapSharedFolders.erase(strName); - /* notify console callbacks after the folder is removed to the list */ + /* Notify console callbacks after the folder is removed from the list. */ + alock.release(); fireSharedFolderChangedEvent(mEventSource, Scope_Session); @@ -4352,5 +4356,8 @@ /* notify console callbacks on success */ if (SUCCEEDED(rc)) + { + alock.release(); /** @todo 101% safe? */ fireNetworkAdapterChangedEvent(mEventSource, aNetworkAdapter); + } LogFlowThisFunc(("Leaving rc=%#x\n", rc)); @@ -4639,6 +4646,4 @@ /** * Called by IInternalSessionControl::OnSerialPortChange(). - * - * @note Locks this object for writing. */ HRESULT Console::onSerialPortChange(ISerialPort *aSerialPort) @@ -4649,28 +4654,12 @@ AssertComRCReturnRC(autoCaller.rc()); - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - HRESULT rc = S_OK; - - /* don't trigger serial port change if the VM isn't running */ - SafeVMPtrQuiet ptrVM(this); - if (ptrVM.isOk()) - { - /* nothing to do so far */ - ptrVM.release(); - } - - /* notify console callbacks on success */ - if (SUCCEEDED(rc)) - fireSerialPortChangedEvent(mEventSource, aSerialPort); - - LogFlowThisFunc(("Leaving rc=%#x\n", rc)); - return rc; + fireSerialPortChangedEvent(mEventSource, aSerialPort); + + LogFlowThisFunc(("Leaving rc=%#x\n", S_OK)); + return S_OK; } /** * Called by IInternalSessionControl::OnParallelPortChange(). - * - * @note Locks this object for writing. */ HRESULT Console::onParallelPortChange(IParallelPort *aParallelPort) @@ -4681,28 +4670,12 @@ AssertComRCReturnRC(autoCaller.rc()); - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - HRESULT rc = S_OK; - - /* don't trigger parallel port change if the VM isn't running */ - SafeVMPtrQuiet ptrVM(this); - if (ptrVM.isOk()) - { - /* nothing to do so far */ - ptrVM.release(); - } - - /* notify console callbacks on success */ - if (SUCCEEDED(rc)) - fireParallelPortChangedEvent(mEventSource, aParallelPort); - - LogFlowThisFunc(("Leaving rc=%#x\n", rc)); - return rc; + fireParallelPortChangedEvent(mEventSource, aParallelPort); + + LogFlowThisFunc(("Leaving rc=%#x\n", S_OK)); + return S_OK; } /** * Called by IInternalSessionControl::OnStorageControllerChange(). - * - * @note Locks this object for writing. */ HRESULT Console::onStorageControllerChange() @@ -4713,22 +4686,8 @@ AssertComRCReturnRC(autoCaller.rc()); - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - HRESULT rc = S_OK; - - /* don't trigger storage controller change if the VM isn't running */ - SafeVMPtrQuiet ptrVM(this); - if (ptrVM.isOk()) - { - /* nothing to do so far */ - ptrVM.release(); - } - - /* notify console callbacks on success */ - if (SUCCEEDED(rc)) - fireStorageControllerChangedEvent(mEventSource); - - LogFlowThisFunc(("Leaving rc=%#x\n", rc)); - return rc; + fireStorageControllerChangedEvent(mEventSource); + + LogFlowThisFunc(("Leaving rc=%#x\n", S_OK)); + return S_OK; } @@ -4759,5 +4718,8 @@ /* notify console callbacks on success */ if (SUCCEEDED(rc)) + { + alock.release(); /** @todo 101% safe? */ fireMediumChangedEvent(mEventSource, aMediumAttachment); + } LogFlowThisFunc(("Leaving rc=%#x\n", rc)); @@ -4833,5 +4795,8 @@ /* notify console callbacks on success */ if (SUCCEEDED(rc)) + { + alock.release(); fireCPUExecutionCapChangedEvent(mEventSource, aExecutionCap); + } LogFlowThisFunc(("Leaving rc=%#x\n", rc)); @@ -4893,12 +4858,12 @@ /* notify console callbacks on success */ if (SUCCEEDED(rc)) + { + alock.release(); fireVRDEServerChangedEvent(mEventSource); + } return rc; } -/** - * @note Locks this object for reading. - */ void Console::onVRDEServerInfoChange() { @@ -4906,6 +4871,4 @@ AssertComRCReturnVoid(autoCaller.rc()); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - fireVRDEServerInfoChangedEvent(mEventSource); } @@ -4914,6 +4877,4 @@ /** * Called by IInternalSessionControl::OnUSBControllerChange(). - * - * @note Locks this object for writing. */ HRESULT Console::onUSBControllerChange() @@ -4924,28 +4885,7 @@ AssertComRCReturnRC(autoCaller.rc()); - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - HRESULT rc = S_OK; - - /* don't trigger USB controller change if the VM isn't running */ - SafeVMPtrQuiet ptrVM(this); - if (ptrVM.isOk()) - { - /// @todo implement one day. - // Anyway, if we want to query the machine's USB Controller we need - // to cache it to mUSBController in #init() (similar to mDVDDrive). - // - // bird: While the VM supports hot-plugging, I doubt any guest can - // handle it at this time... :-) - - /* nothing to do so far */ - ptrVM.release(); - } - - /* notify console callbacks on success */ - if (SUCCEEDED(rc)) - fireUSBControllerChangedEvent(mEventSource); - - return rc; + fireUSBControllerChangedEvent(mEventSource); + + return S_OK; } @@ -4969,4 +4909,5 @@ if (SUCCEEDED(rc)) { + alock.release(); fireSharedFolderChangedEvent(mEventSource, aGlobal ? (Scope_T)Scope_Global : (Scope_T)Scope_Machine); } @@ -5179,5 +5120,8 @@ /* notify console callbacks on success */ if (SUCCEEDED(rc)) + { + alock.release(); fireBandwidthGroupChangedEvent(mEventSource, aBandwidthGroup); + } LogFlowThisFunc(("Leaving rc=%#x\n", rc)); @@ -5214,5 +5158,8 @@ /* notify console callbacks on success */ if (SUCCEEDED(rc)) + { + alock.release(); /** @todo 101% safe? */ fireStorageDeviceChangedEvent(mEventSource, aMediumAttachment, aRemove); + } LogFlowThisFunc(("Leaving rc=%#x\n", rc)); @@ -5685,7 +5632,9 @@ } +#ifdef CONSOLE_WITH_EVENT_CACHE /** * @note Locks this object for writing. */ +#endif void Console::onMousePointerShapeChange(bool fVisible, bool fAlpha, uint32_t xHot, uint32_t yHot, @@ -5702,28 +5651,28 @@ AssertComRCReturnVoid(autoCaller.rc()); -#ifndef CONSOLE_WITH_EVENT_CACHE - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); -#else - /* We need a write lock because we alter the cached callback data */ - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - /* Save the callback arguments */ - mCallbackData.mpsc.visible = fVisible; - mCallbackData.mpsc.alpha = fAlpha; - mCallbackData.mpsc.xHot = xHot; - mCallbackData.mpsc.yHot = yHot; - mCallbackData.mpsc.width = width; - mCallbackData.mpsc.height = height; - - /* start with not valid */ - bool wasValid = mCallbackData.mpsc.valid; - mCallbackData.mpsc.valid = false; - - com::SafeArray aShape(ComSafeArrayInArg(pShape)); - if (aShape.size() != 0) - mCallbackData.mpsc.shape.initFrom(aShape); - else - mCallbackData.mpsc.shape.resize(0); - mCallbackData.mpsc.valid = true; +#ifdef CONSOLE_WITH_EVENT_CACHE + { + /* We need a write lock because we alter the cached callback data */ + AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + + /* Save the callback arguments */ + mCallbackData.mpsc.visible = fVisible; + mCallbackData.mpsc.alpha = fAlpha; + mCallbackData.mpsc.xHot = xHot; + mCallbackData.mpsc.yHot = yHot; + mCallbackData.mpsc.width = width; + mCallbackData.mpsc.height = height; + + /* start with not valid */ + bool wasValid = mCallbackData.mpsc.valid; + mCallbackData.mpsc.valid = false; + + com::SafeArray aShape(ComSafeArrayInArg(pShape)); + if (aShape.size() != 0) + mCallbackData.mpsc.shape.initFrom(aShape); + else + mCallbackData.mpsc.shape.resize(0); + mCallbackData.mpsc.valid = true; + } #endif @@ -5735,7 +5684,9 @@ } +#ifdef CONSOLE_WITH_EVENT_CACHE /** * @note Locks this object for writing. */ +#endif void Console::onMouseCapabilityChange(BOOL supportsAbsolute, BOOL supportsRelative, BOOL needsHostCursor) { @@ -5746,15 +5697,15 @@ AssertComRCReturnVoid(autoCaller.rc()); -#ifndef CONSOLE_WITH_EVENT_CACHE - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); -#else - /* We need a write lock because we alter the cached callback data */ - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - /* save the callback arguments */ - mCallbackData.mcc.supportsAbsolute = supportsAbsolute; - mCallbackData.mcc.supportsRelative = supportsRelative; - mCallbackData.mcc.needsHostCursor = needsHostCursor; - mCallbackData.mcc.valid = true; +#ifdef CONSOLE_WITH_EVENT_CACHE + { + /* We need a write lock because we alter the cached callback data */ + AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + + /* save the callback arguments */ + mCallbackData.mcc.supportsAbsolute = supportsAbsolute; + mCallbackData.mcc.supportsRelative = supportsRelative; + mCallbackData.mcc.needsHostCursor = needsHostCursor; + mCallbackData.mcc.valid = true; + } #endif @@ -5762,7 +5713,4 @@ } -/** - * @note Locks this object for reading. - */ void Console::onStateChange(MachineState_T machineState) { @@ -5770,11 +5718,7 @@ AssertComRCReturnVoid(autoCaller.rc()); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); fireStateChangedEvent(mEventSource, machineState); } -/** - * @note Locks this object for reading. - */ void Console::onAdditionsStateChange() { @@ -5782,15 +5726,13 @@ AssertComRCReturnVoid(autoCaller.rc()); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); fireAdditionsStateChangedEvent(mEventSource); } /** - * @note Locks this object for reading. - * This notification only is for reporting an incompatible - * Guest Additions interface, *not* the Guest Additions version! - * - * The user will be notified inside the guest if new Guest - * Additions are available (via VBoxTray/VBoxClient). + * @remarks This notification only is for reporting an incompatible + * Guest Additions interface, *not* the Guest Additions version! + * + * The user will be notified inside the guest if new Guest + * Additions are available (via VBoxTray/VBoxClient). */ void Console::onAdditionsOutdated() @@ -5799,10 +5741,12 @@ AssertComRCReturnVoid(autoCaller.rc()); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); -} - + /** @todo implement this */ +} + +#ifdef CONSOLE_WITH_EVENT_CACHE /** * @note Locks this object for writing. */ +#endif void Console::onKeyboardLedsChange(bool fNumLock, bool fCapsLock, bool fScrollLock) { @@ -5810,15 +5754,15 @@ AssertComRCReturnVoid(autoCaller.rc()); -#ifndef CONSOLE_WITH_EVENT_CACHE - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); -#else - /* We need a write lock because we alter the cached callback data */ - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - /* save the callback arguments */ - mCallbackData.klc.numLock = fNumLock; - mCallbackData.klc.capsLock = fCapsLock; - mCallbackData.klc.scrollLock = fScrollLock; - mCallbackData.klc.valid = true; +#ifdef CONSOLE_WITH_EVENT_CACHE + { + /* We need a write lock because we alter the cached callback data */ + AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + + /* save the callback arguments */ + mCallbackData.klc.numLock = fNumLock; + mCallbackData.klc.capsLock = fCapsLock; + mCallbackData.klc.scrollLock = fScrollLock; + mCallbackData.klc.valid = true; + } #endif @@ -5826,7 +5770,4 @@ } -/** - * @note Locks this object for reading. - */ void Console::onUSBDeviceStateChange(IUSBDevice *aDevice, bool aAttached, IVirtualBoxErrorInfo *aError) @@ -5835,11 +5776,7 @@ AssertComRCReturnVoid(autoCaller.rc()); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); fireUSBDeviceStateChangedEvent(mEventSource, aDevice, aAttached, aError); } -/** - * @note Locks this object for reading. - */ void Console::onRuntimeError(BOOL aFatal, IN_BSTR aErrorID, IN_BSTR aMessage) { @@ -5847,11 +5784,7 @@ AssertComRCReturnVoid(autoCaller.rc()); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); fireRuntimeErrorEvent(mEventSource, aFatal, aErrorID, aMessage); } -/** - * @note Locks this object for reading. - */ HRESULT Console::onShowWindow(BOOL aCheck, BOOL *aCanShow, LONG64 *aWinId) { @@ -5865,7 +5798,5 @@ AssertComRCReturnRC(autoCaller.rc()); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); VBoxEventDesc evDesc; - if (aCheck) { @@ -5887,5 +5818,5 @@ else { - Assert(FALSE); + AssertFailed(); *aCanShow = TRUE; } @@ -5904,13 +5835,13 @@ evDesc.getEvent(pEvent.asOutParam()); ComPtr pShowEvent = pEvent; - LONG64 aEvWinId = 0; if (pShowEvent) { - pShowEvent->COMGETTER(WinId)(&aEvWinId); - if ((aEvWinId != 0) && (*aWinId == 0)) - *aWinId = aEvWinId; + LONG64 iEvWinId = 0; + pShowEvent->COMGETTER(WinId)(&iEvWinId); + if (iEvWinId != 0 && *aWinId == 0) + *aWinId = iEvWinId; } else - Assert(FALSE); + AssertFailed(); } }