Changeset 27781 in vbox

Timestamp:

Mar 29, 2010 11:52:20 AM (15 years ago)

Author:

vboxsync

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

59430

Message:

HostServices/GuestProperties: get rid of the notification thread (see XTracker #3201 comment 91)

Location:

trunk

Files:

• include/VBox/HostServices/GuestPropertySvc.h (modified) (1 diff)
• src/VBox/HostServices/GuestProperties/service.cpp (modified) (14 diffs)

trunk/include/VBox/HostServices/GuestPropertySvc.h

@@ -251 +251 @@
      * Takes one 32-bit unsigned integer parameter for the flags.
      */
-    SET_GLOBAL_FLAGS_HOST = 7,
-
-    /**
-     * Flush notifications.
-     * Takes one 32-bit unsigned integer parameter that gives the number of
-     * milliseconds to wait for the worker thread to get the work done.
-     */
-    FLUSH_NOTIFICATIONS_HOST
+    SET_GLOBAL_FLAGS_HOST = 7
 };
 

trunk/src/VBox/HostServices/GuestProperties/service.cpp

@@ -163 +163 @@
     CallList mGuestWaiters;
     /** @todo we should have classes for thread and request handler thread */
-    /** Queue of outstanding property change notifications */
-    RTREQQUEUE *mReqQueue;
-    /** Request that we've left pending in a call to flushNotifications. */
-    PRTREQ mPendingDummyReq;
-    /** Thread for processing the request queue */
-    RTTHREAD mReqThread;
-    /** Tell the thread that it should exit */
-    bool volatile mfExitThread;
     /** Callback function supplied by the host for notification of updates
      * to properties */
@@ -249 +241 @@
         : mpHelpers(pHelpers)
         , meGlobalFlags(NILFLAG)
-        , mPendingDummyReq(NULL)
-        , mfExitThread(false)
         , mpfnHostCallback(NULL)
         , mpvHostData(NULL)
-    {
-        int rc = RTReqCreateQueue(&mReqQueue);
-#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
-        if (RT_SUCCESS(rc))
-            rc = RTThreadCreate(&mReqThread, reqThreadFn, this, 0,
-                                RTTHREADTYPE_MSG_PUMP, RTTHREADFLAGS_WAITABLE,
-                                "GuestPropReq");
-#endif
-        if (RT_FAILURE(rc))
-            throw rc;
-    }
+    { }
 
     /**
@@ -350 +330 @@
     int delProperty(uint32_t cParms, VBOXHGCMSVCPARM paParms[], bool isGuest);
     int enumProps(uint32_t cParms, VBOXHGCMSVCPARM paParms[]);
-    int flushNotifications(uint32_t cMsTimeout);
     int getNotification(VBOXHGCMCALLHANDLE callHandle, uint32_t cParms,
                         VBOXHGCMSVCPARM paParms[]);
@@ -357 +336 @@
     int getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop);
     void doNotifications(const char *pszProperty, uint64_t u64Timestamp);
-    static DECLCALLBACK(int) reqNotify(PFNHGCMSVCEXT pfnCallback,
-                                       void *pvData, char *pszName,
-                                       char *pszValue, uint32_t u32TimeHigh,
-                                       uint32_t u32TimeLow, char *pszFlags);
-    /**
-     * Empty request function for terminating the request thread.
-     * @returns VINF_EOF to cause the request processing function to return
-     * @todo    return something more appropriate
-     */
-    static DECLCALLBACK(int) reqVoid() { return VINF_EOF; }
+    int notifyHost(const char *pszName, const char *pszValue,
+                   uint64_t u64Timestamp, const char *pszFlags);
 
     void call (VBOXHGCMCALLHANDLE callHandle, uint32_t u32ClientID,
@@ -374 +345 @@
     int uninit ();
 };
-
-
-/**
- * Thread function for processing the request queue
- * @copydoc FNRTTHREAD
- */
-/* static */
-DECLCALLBACK(int) Service::reqThreadFn(RTTHREAD ThreadSelf, void *pvUser)
-{
-    SELF *pSelf = reinterpret_cast<SELF *>(pvUser);
-    while (!pSelf->mfExitThread)
-        RTReqProcess(pSelf->mReqQueue, RT_INDEFINITE_WAIT);
-    return VINF_SUCCESS;
-}
 
 
@@ -849 +806 @@
 }
 
-/**
- * Flushes the notifications.
- *
- * @returns iprt status value
- * @param   cMsTimeout  The timeout in milliseconds.
- * @thread  HGCM
- */
-int Service::flushNotifications(uint32_t cMsTimeout)
-{
-    LogFlowThisFunc(("cMsTimeout=%RU32\n", cMsTimeout));
-    int rc;
-
-#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
-    /*
-     * Wait for the thread to finish processing all current requests.
-     */
-    if (!mPendingDummyReq && !RTReqIsBusy(mReqQueue))
-        rc = VINF_SUCCESS;
-    else
-    {
-        if (!mPendingDummyReq)
-            rc = RTReqCallEx(mReqQueue, &mPendingDummyReq, 0 /*cMillies*/, RTREQFLAGS_VOID, (PFNRT)reqVoid, 0);
-        else
-            rc = VERR_TIMEOUT;
-        if (rc == VERR_TIMEOUT)
-            rc = RTReqWait(mPendingDummyReq, cMsTimeout);
-        if (RT_SUCCESS(rc))
-        {
-            RTReqFree(mPendingDummyReq);
-            mPendingDummyReq = NULL;
-        }
-    }
-#else
-    NOREF(cMsTimeout);
-    rc = VINF_SUCCESS;
-#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
-
-    return rc;
-}
 
 /** Helper query used by getOldNotification */
@@ -919 +837 @@
 }
 
+
 /** Helper query used by getNotification */
 int Service::getNotificationWriteOut(VBOXHGCMSVCPARM paParms[], Property prop)
@@ -957 +876 @@
 }
 
+
 /**
  * Get the next guest notification.
@@ -1015 +935 @@
 }
 
+
 /**
  * Notify the service owner and the guest that a property has been
@@ -1020 +941 @@
  * @param pszProperty  the name of the property which has changed
  * @param u64Timestamp the time at which the change took place
- * @note this call allocates memory which the reqNotify request is expected to
- *       free again, using RTStrFree().
  *
  * @thread  HGCM service
@@ -1091 +1010 @@
         mGuestNotifications.pop_front();
 
-#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
     /*
      * Host notifications - first case: if the property exists then send its
      * current value
      */
-    char *pszName = NULL, *pszValue = NULL, *pszFlags = NULL;
-
     if (found && mpfnHostCallback != NULL)
     {
         char szFlags[MAX_FLAGS_LEN];
         /* Send out a host notification */
-        rc = writeFlags(prop.mFlags, szFlags);
+        const char *pszValue = prop.mValue.c_str();
         if (RT_SUCCESS(rc))
-            rc = RTStrDupEx(&pszName, pszProperty);
+            rc = writeFlags(prop.mFlags, szFlags);
         if (RT_SUCCESS(rc))
-            rc = RTStrDupEx(&pszValue, prop.mValue.c_str());
-        if (RT_SUCCESS(rc))
-            rc = RTStrDupEx(&pszFlags, szFlags);
-        if (RT_SUCCESS(rc))
-        {
-            LogFlowThisFunc (("pszName=%p (%s)\n", pszName, pszName));
-            LogFlowThisFunc (("pszValue=%p (%s)\n", pszValue, pszValue));
-            LogFlowThisFunc (("pszFlags=%p (%s)\n", pszFlags, pszFlags));
-            rc = RTReqCallEx(mReqQueue, NULL, 0, RTREQFLAGS_NO_WAIT,
-                             (PFNRT)Service::reqNotify, 7, mpfnHostCallback,
-                             mpvHostData, pszName, pszValue,
-                             (uint32_t) RT_HIDWORD(u64Timestamp),
-                             (uint32_t) RT_LODWORD(u64Timestamp), pszFlags);
-        }
+            rc = notifyHost(pszProperty, pszValue, u64Timestamp, szFlags);
     }
 
@@ -1129 +1032 @@
     {
         /* Send out a host notification */
-        rc = RTStrDupEx(&pszName, pszProperty);
         if (RT_SUCCESS(rc))
-        {
-            LogFlowThisFunc (("pszName=%p (%s)\n", pszName, pszName));
-            rc = RTReqCallEx(mReqQueue, NULL, 0, RTREQFLAGS_NO_WAIT,
-                             (PFNRT)Service::reqNotify, 7, mpfnHostCallback,
-                             mpvHostData, pszName, (uintptr_t) NULL,
-                             (uint32_t) RT_HIDWORD(u64Timestamp),
-                             (uint32_t) RT_LODWORD(u64Timestamp),
-                             (uintptr_t) NULL);
-       }
-    }
-    if (RT_FAILURE(rc)) /* clean up if we failed somewhere */
-    {
-        LogThisFunc (("Failed, freeing allocated strings.\n"));
-        RTStrFree(pszName);
-        RTStrFree(pszValue);
-        RTStrFree(pszFlags);
+            rc = notifyHost(pszProperty, NULL, u64Timestamp, NULL);
     }
     LogFlowThisFunc (("returning\n"));
-#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
 }
 
 /**
  * Notify the service owner that a property has been added/deleted/changed.
- * asynchronous part.
- * @param pszProperty the name of the property which has changed
- * @note this call allocates memory which the reqNotify request is expected to
- *       free again, using RTStrFree().
- *
- * @thread  request thread
- */
-/* static */
-DECLCALLBACK(int) Service::reqNotify(PFNHGCMSVCEXT pfnCallback, void *pvData,
-                                     char *pszName, char *pszValue, uint32_t u32TimeHigh,
-                                     uint32_t u32TimeLow, char *pszFlags)
-{
-    LogFlowFunc (("pfnCallback=%p, pvData=%p, pszName=%p, pszValue=%p, u32TimeHigh=%u, u32TimeLow=%u, pszFlags=%p\n", pfnCallback, pvData, pszName, pszValue, u32TimeHigh, u32TimeLow, pszFlags));
-    LogFlowFunc (("pszName=%s\n", pszName));
-    LogFlowFunc (("pszValue=%s\n", pszValue));
-    LogFlowFunc (("pszFlags=%s\n", pszFlags));
-    /* LogFlowFunc (("pfnCallback=%p, pvData=%p, pszName=%s, pszValue=%s, u32TimeHigh=%u, u32TimeLow=%u, pszFlags=%s\n", pfnCallback, pvData, pszName, pszValue, u32TimeHigh, u32TimeLow, pszFlags)); */
+ * @returns  IPRT status value
+ * @param    pszName       the property name
+ * @param    pszValue      the new value, or NULL if the property was deleted
+ * @param    u64Timestamp  the time of the change
+ * @param    pszFlags      the new flags string
+ */
+int Service::notifyHost(const char *pszName, const char *pszValue,
+                        uint64_t u64Timestamp, const char *pszFlags)
+{
+    LogFlowFunc (("pszName=%s, pszValue=%s, u64Timestamp=%llu, pszFlags=%s\n",
+                  pszName, pszValue, u64Timestamp, pszFlags));
     HOSTCALLBACKDATA HostCallbackData;
     HostCallbackData.u32Magic     = HOSTCALLBACKMAGIC;
     HostCallbackData.pcszName     = pszName;
     HostCallbackData.pcszValue    = pszValue;
-    HostCallbackData.u64Timestamp = RT_MAKE_U64(u32TimeLow, u32TimeHigh);
+    HostCallbackData.u64Timestamp = u64Timestamp;
     HostCallbackData.pcszFlags    = pszFlags;
-    int rc = pfnCallback(pvData, 0 /*u32Function*/, (void *)(&HostCallbackData),
-                         sizeof(HostCallbackData));
-    AssertRC(rc);
-    LogFlowFunc (("Freeing strings\n"));
-    RTStrFree(pszName);
-    RTStrFree(pszValue);
-    RTStrFree(pszFlags);
-    LogFlowFunc (("returning success\n"));
-    return VINF_SUCCESS;
+    int rc = mpfnHostCallback (mpvHostData, 0 /*u32Function*/,
+                           (void *)(&HostCallbackData),
+                           sizeof(HostCallbackData));
+    LogFlowFunc (("returning %Rrc\n", rc));
+    return rc;
 }
 
@@ -1328 +1204 @@
                 break;
 
-            /* The host wishes to flush all pending notification */
-            case FLUSH_NOTIFICATIONS_HOST:
-                LogFlowFunc(("FLUSH_NOTIFICATIONS_HOST\n"));
-                if (cParms == 1)
-                {
-                    uint32_t cMsTimeout;
-                    rc = paParms[0].getUInt32(&cMsTimeout);
-                    if (RT_SUCCESS(rc))
-                        rc = flushNotifications(cMsTimeout);
-                }
-                else
-                    rc = VERR_INVALID_PARAMETER;
-                break;
-
             default:
                 rc = VERR_NOT_SUPPORTED;
@@ -1358 +1220 @@
 int Service::uninit()
 {
-    int rc = VINF_SUCCESS;
-
-    ASMAtomicWriteBool(&mfExitThread, true);
-
-#ifndef VBOX_GUEST_PROP_TEST_NOTHREAD
-    /*
-     * Send a dummy request to the thread so it is forced out of the loop and
-     * notice that the exit flag is set.  Give up waiting after 5 mins.
-     * We call flushNotifications first to try clean up any pending request.
-     */
-    flushNotifications(120*1000);
-
-    rc = RTReqCallEx(mReqQueue, NULL, 0, RTREQFLAGS_NO_WAIT, (PFNRT)reqVoid, 0);
-    if (RT_SUCCESS(rc))
-    {
-        unsigned count = 0;
-        do
-        {
-            rc = RTThreadWait(mReqThread, 1000, NULL);
-            ++count;
-            Assert(RT_SUCCESS(rc) || ((VERR_TIMEOUT == rc) && (count != 5)));
-        } while ((VERR_TIMEOUT == rc) && (count < 300));
-    }
-#endif /* VBOX_GUEST_PROP_TEST_NOTHREAD not defined */
-    if (RT_SUCCESS(rc))
-        RTReqDestroyQueue(mReqQueue);
-    return rc;
+    return VINF_SUCCESS;
 }