Changeset 76066 in vbox

Timestamp:

Dec 7, 2018 10:17:39 PM (6 years ago)

Author:

vboxsync

Message:

VBoxSDS: updates

Location:

trunk/src/VBox/Main

Files:

• include/VirtualBoxClientListImpl.h (modified) (5 diffs)
• include/VirtualBoxSDSImpl.h (modified) (2 diffs)
• src-global/VirtualBoxSDSImpl.cpp (modified) (7 diffs)
• src-global/win/VirtualBoxClientListImpl.cpp (modified) (13 diffs)
• src-server/win/svcmain.cpp (modified) (1 diff)

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

@@ +1 @@
+/* $Id$ */
 /** @file
-* VBox Global COM Class definition
-*/
+ * Main - VBoxSDS - VirtualBoxClientList.
+ */
 
 /*
@@ -27 +28 @@
 #include "VirtualBoxClientListWrap.h"
 
-/**
-* The IVirtualBoxClientList implementation.
-*
-* This class provides COM interface to track and get list of
-* API client processes.
-*
-*/
-
 typedef std::set<LONG> 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 +50 @@
 
 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 +65 @@
     // Private members
     // polling of unexpectedly finished api client processes
-    CClientListWatcher* m_pWatcher;
+    CClientListWatcher *m_pWatcher;
 };
 
@@ -72 +71 @@
 
 #endif // !____H_VIRTUALBOXCLIENTLISTIMPL
+

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

@@ -71 +71 @@
 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 +102 @@
      */
     VBoxSDSPerUserData *i_lookupOrCreatePerUserData(com::Utf8Str const &a_rStrUserSid, com::Utf8Str const &a_rStrUsername);
+    /** @} */
 };
 

trunk/src/VBox/Main/src-global/VirtualBoxSDSImpl.cpp

@@ -165 +165 @@
             {
                 /*
-                 * 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 +182 @@
                     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 +202 @@
                     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 +235 @@
 }
 
+
 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 +255 @@
                             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 +290 @@
     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 +296 @@
             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();
             }

trunk/src/VBox/Main/src-global/win/VirtualBoxClientListImpl.cpp

@@ -17 +17 @@
 
 
+/*********************************************************************************************************************************
+*   Header Files                                                                                                                 *
+*********************************************************************************************************************************/
 #include "Logging.h"
 #include "VirtualBoxClientListImpl.h"
@@ -24 +27 @@
 
 
-////////////////// CClientListWatcher implementation /////////////////
+/*********************************************************************************************************************************
+*   CClientListWatcher                                                                                                           *
+*********************************************************************************************************************************/
 
 /**
@@ -41 +46 @@
 
 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 +73 @@
     }
 
-    int rc = RTSemEventCreate(&m_wakeUpWatcherEvent);
+    int rc = RTSemEventCreate(&m_hEvtWakeUpWatcher);
     if (RT_FAILURE(rc))
     {
@@ -71 +80 @@
     }
 
-    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 +92 @@
     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 +109 @@
 
     // 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 +118 @@
         LogRelFunc(("Watcher thread finished.\n"));
 
-    ASMAtomicWriteNullPtr((void* volatile*)&CClientListWatcher::s_WatcherThread);
-
-    RTSemEventDestroy(m_wakeUpWatcherEvent);
+    ASMAtomicWriteHandle(&CClientListWatcher::s_hThreadWatcher, NIL_RTTHREAD);
+
+    RTSemEventDestroy(m_hEvtWakeUpWatcher);
 }
 
@@ -121 +134 @@
 {
     ComPtr<IVirtualBoxSDS> ptrVirtualBoxSDS;
+
     /*
-    * Connect to VBoxSDS.
-    */
+     * Connect to VBoxSDS.
+     */
     HRESULT hrc = CoCreateInstance(CLSID_VirtualBoxSDS, NULL, CLSCTX_LOCAL_SERVER, IID_IVirtualBoxSDS,
                                    (void **)ptrVirtualBoxSDS.asOutParam());
@@ -136 +150 @@
 /**
  * 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 +200 @@
         }
 
-        rc = RTCritSectRwLeaveShared(&pThis->m_clientListCritSect);
+        rc = RTCritSectRwLeaveShared(&pThis->m_rClientListCritSect);
         Assert(RT_SUCCESS(rc));
 
@@ -205 +210 @@
  * 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 +311 @@
             aPids.assign(m_ClientSet.begin(), m_ClientSet.end());
         }
-        catch (std::bad_alloc)
+        catch (std::bad_alloc &)
         {
             RTCritSectRwLeaveShared(&m_MapCritSect);

trunk/src/VBox/Main/src-server/win/svcmain.cpp

@@ -443 +443 @@
     {
         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;
     }