Index: /trunk/src/VBox/Main/include/VirtualBoxClientListImpl.h =================================================================== --- /trunk/src/VBox/Main/include/VirtualBoxClientListImpl.h (revision 76065) +++ /trunk/src/VBox/Main/include/VirtualBoxClientListImpl.h (revision 76066) @@ -1,5 +1,6 @@ +/* $Id$ */ /** @file -* VBox Global COM Class definition -*/ + * Main - VBoxSDS - VirtualBoxClientList. + */ /* @@ -27,16 +28,14 @@ #include "VirtualBoxClientListWrap.h" -/** -* The IVirtualBoxClientList implementation. -* -* This class provides COM interface to track and get list of -* API client processes. -* -*/ - typedef std::set TClientSet; class CClientListWatcher; +/** + * The IVirtualBoxClientList implementation. + * + * This class provides COM interface to track and get list of + * API client processes. + */ class ATL_NO_VTABLE VirtualBoxClientList : public VirtualBoxClientListWrap @@ -51,5 +50,5 @@ public: - DECLARE_CLASSFACTORY_SINGLETON(VirtualBoxClientList) + DECLARE_CLASSFACTORY_SINGLETON(VirtualBoxClientList) /**< r=bird: It is _NOT_ a singleton. */ DECLARE_NOT_AGGREGATABLE(VirtualBoxClientList) VirtualBoxClientList() : m_pWatcher(NULL) {} @@ -66,5 +65,5 @@ // Private members // polling of unexpectedly finished api client processes - CClientListWatcher* m_pWatcher; + CClientListWatcher *m_pWatcher; }; @@ -72,2 +71,3 @@ #endif // !____H_VIRTUALBOXCLIENTLISTIMPL + Index: /trunk/src/VBox/Main/include/VirtualBoxSDSImpl.h =================================================================== --- /trunk/src/VBox/Main/include/VirtualBoxSDSImpl.h (revision 76065) +++ /trunk/src/VBox/Main/include/VirtualBoxSDSImpl.h (revision 76066) @@ -71,12 +71,14 @@ private: - // IVirtualBoxSDS methods + /** @name IVirtualBoxSDS methods + * @{ */ STDMETHOD(RegisterVBoxSVC)(IVBoxSVCRegistration *aVBoxSVC, LONG aPid, IUnknown **aExistingVirtualBox); STDMETHOD(DeregisterVBoxSVC)(IVBoxSVCRegistration *aVBoxSVC, LONG aPid); - STDMETHOD(NotifyClientsFinished)(); + STDMETHOD(NotifyClientsFinished)(); /**< @todo r=bird: Internal within SDS! Make private. */ + /** @} */ - // Private methods - + /** @name Private methods + * @{ */ /** * Gets the client user SID of the @@ -100,4 +102,5 @@ */ VBoxSDSPerUserData *i_lookupOrCreatePerUserData(com::Utf8Str const &a_rStrUserSid, com::Utf8Str const &a_rStrUsername); + /** @} */ }; Index: /trunk/src/VBox/Main/src-global/VirtualBoxSDSImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-global/VirtualBoxSDSImpl.cpp (revision 76065) +++ /trunk/src/VBox/Main/src-global/VirtualBoxSDSImpl.cpp (revision 76066) @@ -165,6 +165,7 @@ { /* - * If there already is a chosen one, check that it is still around, - * replace it with the caller if no response. + * If there already is a chosen one, ask it for a IVirtualBox instance + * to return to the caller. Should it be dead or unresponsive, the caller + * takes its place. */ if (pUserData->m_ptrTheChosenOne.isNotNull()) @@ -181,5 +182,5 @@ if (FAILED_DEAD_INTERFACE(hrc)) { - LogRel(("VirtualBoxSDS::registerVBoxSVC: Seems VBoxSVC instance died. Dropping it and letting caller take over.\n")); + LogRel(("VirtualBoxSDS::registerVBoxSVC: Seems VBoxSVC instance died. Dropping it and letting caller take over. (hrc=%Rhrc)\n", hrc)); pUserData->m_ptrTheChosenOne.setNull(); @@ -201,11 +202,7 @@ try { -#if 1 - hrc = pUserData->m_ptrClientList.createLocalObject(CLSID_VirtualBoxClientList); -#else - hrc = CoCreateInstance(CLSID_VirtualBoxClientList, NULL, CLSCTX_LOCAL_SERVER, - IID_IVirtualBoxClientList, - (void **)pUserData->m_ptrClientList.asOutParam()); -#endif + /// @todo r=bird: Enable when design has been corrected. + ///hrc = pUserData->m_ptrClientList.createLocalObject(CLSID_VirtualBoxClientList); + hrc = S_OK; if (SUCCEEDED(hrc)) { @@ -238,7 +235,8 @@ } + STDMETHODIMP VirtualBoxSDS::DeregisterVBoxSVC(IVBoxSVCRegistration *aVBoxSVC, LONG aPid) { - LogRel(("VirtualBoxSDS::deregisterVBoxSVC: aVBoxSVC=%p aPid=%u\n", (IVBoxSVCRegistration *)aVBoxSVC, aPid)); + LogRel(("VirtualBoxSDS::deregisterVBoxSVC: aVBoxSVC=%p aPid=%u (%#x)\n", (IVBoxSVCRegistration *)aVBoxSVC, aPid, aPid)); HRESULT hrc; if (RT_VALID_PTR(aVBoxSVC)) @@ -257,5 +255,4 @@ pUserData->m_strUserSid.c_str(), pUserData->m_strUsername.c_str())); pUserData->m_ptrTheChosenOne.setNull(); - /** @todo consider evicting the user from the table... */ /* Release the client list and stop client list watcher thread*/ pUserData->m_ptrClientList.setNull(); @@ -293,4 +290,5 @@ if (RT_SUCCESS(vrc)) { +/** @todo r=bird: Why notify all VBoxSVC instances? That makes zero sense! */ for (UserDataMap_T::iterator it = m_UserDataMap.begin(); it != m_UserDataMap.end(); ++it) { @@ -298,9 +296,12 @@ if (pUserData && pUserData->m_ptrTheChosenOne) { + /* + * Notify VBoxSVC about finishing all API clients it should free + * references to VBoxSDS and clean up itself + */ LogRelFunc(("Notify VBoxSVC that all clients finished\n")); - /* Notify VBoxSVC about finishing all API clients it should free references to VBoxSDS - and clean up itself */ - if (pUserData->m_ptrClientList) + if (pUserData->m_ptrClientList.isNotNull()) pUserData->m_ptrClientList.setNull(); + pUserData->m_ptrTheChosenOne->NotifyClientsFinished(); } Index: /trunk/src/VBox/Main/src-global/win/VirtualBoxClientListImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-global/win/VirtualBoxClientListImpl.cpp (revision 76065) +++ /trunk/src/VBox/Main/src-global/win/VirtualBoxClientListImpl.cpp (revision 76066) @@ -17,4 +17,7 @@ +/********************************************************************************************************************************* +* Header Files * +*********************************************************************************************************************************/ #include "Logging.h" #include "VirtualBoxClientListImpl.h" @@ -24,5 +27,7 @@ -////////////////// CClientListWatcher implementation ///////////////// +/********************************************************************************************************************************* +* CClientListWatcher * +*********************************************************************************************************************************/ /** @@ -41,22 +46,26 @@ protected: - static DECLCALLBACK(int) WatcherWorker(RTTHREAD ThreadSelf, void *pvUser); + static DECLCALLBACK(int) WatcherThreadProc(RTTHREAD hThreadSelf, void *pvUser); void NotifySDSAllClientsFinished(); - // s_WatcherThread is static to check that single watcher thread used only - static volatile RTTHREAD s_WatcherThread; + volatile bool m_fWatcherRunning; - TClientSet& m_clientList; - RTCRITSECTRW& m_clientListCritSect; - RTSEMEVENT m_wakeUpWatcherEvent; + TClientSet &m_clientList; + RTCRITSECTRW &m_rClientListCritSect; + RTSEMEVENT m_hEvtWakeUpWatcher; + + /** There is a single watcher thread. */ + static volatile RTTHREAD s_hThreadWatcher; }; -volatile RTTHREAD CClientListWatcher::s_WatcherThread = NULL; - -CClientListWatcher::CClientListWatcher(TClientSet& list, RTCRITSECTRW& clientListCritSect) - : m_clientList(list), m_clientListCritSect(clientListCritSect) -{ - Assert(ASMAtomicReadPtr((void* volatile*)&CClientListWatcher::s_WatcherThread) == NULL); - - if (ASMAtomicReadPtr((void* volatile*)&CClientListWatcher::s_WatcherThread) != NULL) + +volatile RTTHREAD CClientListWatcher::s_hThreadWatcher = NIL_RTTHREAD; + + +CClientListWatcher::CClientListWatcher(TClientSet &list, RTCRITSECTRW &rClientListCritSect) + : m_clientList(list), m_rClientListCritSect(rClientListCritSect) +{ + Assert(ASMAtomicReadPtr((void * volatile *)&CClientListWatcher::s_hThreadWatcher) == NULL); + + if (ASMAtomicReadPtr((void * volatile *)&CClientListWatcher::s_hThreadWatcher) != NULL) { LogRelFunc(("Error: Watcher thread already created!\n")); @@ -64,5 +73,5 @@ } - int rc = RTSemEventCreate(&m_wakeUpWatcherEvent); + int rc = RTSemEventCreate(&m_hEvtWakeUpWatcher); if (RT_FAILURE(rc)) { @@ -71,7 +80,8 @@ } - RTTHREAD watcherThread; - rc = RTThreadCreate(&watcherThread, - (PFNRTTHREAD)CClientListWatcher::WatcherWorker, + ASMAtomicWriteBool(&m_fWatcherRunning, true); + RTTHREAD hThreadWatcher; + rc = RTThreadCreate(&hThreadWatcher, + (PFNRTTHREAD)CClientListWatcher::WatcherThreadProc, this, // pVUser 0, // cbStack @@ -82,9 +92,12 @@ if (RT_SUCCESS(rc)) { - ASMAtomicWritePtr((void* volatile*)&CClientListWatcher::s_WatcherThread, watcherThread); + ASMAtomicWriteHandle(&CClientListWatcher::s_hThreadWatcher, hThreadWatcher); LogRelFunc(("Created client list watcher thread.\n")); } else + { + ASMAtomicWriteBool(&m_fWatcherRunning, false); LogRelFunc(("Failed to create client list watcher thread: %Rrs\n", rc)); + } } @@ -96,8 +109,8 @@ // Wake up watcher thread to finish it faster - int rc = RTSemEventSignal(m_wakeUpWatcherEvent); - Assert(RT_SUCCESS(rc)); - - rc = RTThreadWait(CClientListWatcher::s_WatcherThread, RT_INDEFINITE_WAIT, NULL); + int rc = RTSemEventSignal(m_hEvtWakeUpWatcher); + AssertRC(rc); + + rc = RTThreadWait(CClientListWatcher::s_hThreadWatcher, RT_INDEFINITE_WAIT, NULL); if (RT_FAILURE(rc)) LogRelFunc(("Error: watcher thread didn't finished. Possible thread leak. %Rrs\n", rc)); @@ -105,7 +118,7 @@ LogRelFunc(("Watcher thread finished.\n")); - ASMAtomicWriteNullPtr((void* volatile*)&CClientListWatcher::s_WatcherThread); - - RTSemEventDestroy(m_wakeUpWatcherEvent); + ASMAtomicWriteHandle(&CClientListWatcher::s_hThreadWatcher, NIL_RTTHREAD); + + RTSemEventDestroy(m_hEvtWakeUpWatcher); } @@ -121,7 +134,8 @@ { ComPtr ptrVirtualBoxSDS; + /* - * Connect to VBoxSDS. - */ + * Connect to VBoxSDS. + */ HRESULT hrc = CoCreateInstance(CLSID_VirtualBoxSDS, NULL, CLSCTX_LOCAL_SERVER, IID_IVirtualBoxSDS, (void **)ptrVirtualBoxSDS.asOutParam()); @@ -136,26 +150,17 @@ /** * Deregister all staled VBoxSVC through VBoxSDS and forcebly close VBoxSVC process - * @param ThreadSelf current thread id + * @param hThreadSelf The thread handle. * @param pvUser pointer to CClientListWatcher that created this thread. */ -DECLCALLBACK(int) CClientListWatcher::WatcherWorker(RTTHREAD ThreadSelf, void *pvUser) -{ - NOREF(ThreadSelf); - /** @todo r=bird: This will fail once in a while because you don't know - * for sure how the scheduling is going to be. So, RTThreadCreate - * may return and set g_hWatcherThread after the thread started - * executing and got here! */ - Assert(ASMAtomicReadPtr((void* volatile*)&CClientListWatcher::s_WatcherThread)); - LogRelFunc(("Enter watcher thread\n")); - +DECLCALLBACK(int) CClientListWatcher::WatcherThreadProc(RTTHREAD hThreadSelf, void *pvUser) +{ + LogRelFunc(("Enter watcher thread (%p)\n", hThreadSelf)); NOREF(hThreadSelf); CClientListWatcher *pThis = (CClientListWatcher *)pvUser; Assert(pThis); - ASMAtomicWriteBool(&pThis->m_fWatcherRunning, true); - while (ASMAtomicReadBool(&pThis->m_fWatcherRunning)) { /* remove finished API clients from list */ - int rc = RTCritSectRwEnterShared(&pThis->m_clientListCritSect); + int rc = RTCritSectRwEnterShared(&pThis->m_rClientListCritSect); Assert(RT_SUCCESS(rc)); NOREF(rc); @@ -195,5 +200,5 @@ } - rc = RTCritSectRwLeaveShared(&pThis->m_clientListCritSect); + rc = RTCritSectRwLeaveShared(&pThis->m_rClientListCritSect); Assert(RT_SUCCESS(rc)); @@ -205,9 +210,15 @@ * you could wait on the first 63 client processes here in addition to the event. * That would speed up the response time. */ - RTSemEventWait(pThis->m_wakeUpWatcherEvent, 2000); + RTSemEventWait(pThis->m_hEvtWakeUpWatcher, 2000); } LogRelFunc(("Finish watcher thread. Client list size: %d\n", pThis->m_clientList.size())); return 0; } + + + +/********************************************************************************************************************************* +* VirtualBoxClientList * +*********************************************************************************************************************************/ ///////////////////////// VirtualBoxClientList implementation ////////////////////////// @@ -300,5 +311,5 @@ aPids.assign(m_ClientSet.begin(), m_ClientSet.end()); } - catch (std::bad_alloc) + catch (std::bad_alloc &) { RTCritSectRwLeaveShared(&m_MapCritSect); Index: /trunk/src/VBox/Main/src-server/win/svcmain.cpp =================================================================== --- /trunk/src/VBox/Main/src-server/win/svcmain.cpp (revision 76065) +++ /trunk/src/VBox/Main/src-server/win/svcmain.cpp (revision 76066) @@ -443,12 +443,15 @@ { LogRelFunc(("All clients gone - shutdown sequence initiated\n")); - if(m_pFactory) + if (m_pFactory) m_pFactory->i_finishVBoxSvc(); // This is not enough to finish VBoxSvc such as reference to crashed client still is in action // So I forcebly shutdown VBoxSvc - while (g_pModule->Unlock() > 0) - {}; - + LONG cLocks = g_pModule->Unlock(); + LogRelFunc(("Unlock -> %d\n", cLocks)); + while (cLocks > 0) + cLocks = g_pModule->Unlock(); + + LogRelFunc(("returns\n")); return S_OK; }