]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commitdiff
Add a monotonic glib timer to Tools.
authorVMware, Inc <>
Tue, 24 Aug 2010 18:45:10 +0000 (11:45 -0700)
committerMarcelo Vanzin <mvanzin@vmware.com>
Tue, 24 Aug 2010 18:45:10 +0000 (11:45 -0700)
While investigating bug 600636, I noticed that glib timers sometimes would
get into a funny state where they'd ask the main loop to sleep for a very
short time, even though they weren't ready to be fired. I couldn't figure
out why they were doing that, but anyway, we can't easily modify glib.

Instead, add a new timer, based on a monotonic clock, to use in preference
to the glib timer. This is more important for quick firing timers such as
the RPC loop or the C&P pointer poll than it is for coarser-grained timers
such as the guest info gather loop.

Along with it, change the rpcin library and the dnd plugin to use the new
timer in place of the glib one, and avoid re-creating the timer on every
iteration of the rpcin loop - do it only when the delay has changed.

I was only able to reproduce the original issue on a 32-bit Win XP VM. I
tried a 64-bit Win 7 VM and a 64-bit Linux VM, and both seemed to behave
fine.

Signed-off-by: Marcelo Vanzin <mvanzin@vmware.com>
open-vm-tools/lib/include/vmware/tools/utils.h
open-vm-tools/lib/rpcIn/rpcin.c
open-vm-tools/libvmtools/Makefile.am
open-vm-tools/libvmtools/monotonicTimer.c [new file with mode: 0644]
open-vm-tools/services/plugins/dndcp/copyPasteDnDWrapper.cpp
open-vm-tools/services/plugins/dndcp/pointer.cpp

index ca837b3650eb4c0eefc02f4b5f7b8b9282f39952..a64899e6d76ff52c2657f30d7415786b41691f22 100644 (file)
@@ -129,6 +129,9 @@ VMTools_NewSignalSource(int signum);
 
 #endif
 
+GSource *
+VMTools_CreateTimer(gint timeout);
+
 GArray *
 VMTools_WrapArray(gconstpointer data,
                   guint elemSize,
index eec99424430ad6c27052a2f1fa4f842fcd0b4b03..588e7c9aab2a408e587a6b2b7314dcf00209f034 100644 (file)
@@ -46,6 +46,7 @@
 
 #if defined(VMTOOLS_USE_GLIB)
 #  include "vmware/tools/guestrpc.h"
+#  include "vmware/tools/utils.h"
 #endif
 
 #include "vmware.h"
@@ -589,16 +590,15 @@ RpcInLoop(void *clientData) // IN
    char const *errmsg;
    char const *reply;
    size_t repLen;
+   Bool resched = FALSE;
+   unsigned int current;
 
    in = (RpcIn *)clientData;
    ASSERT(in);
 
    /* The event has fired: it is no longer valid */
    ASSERT(in->nextEvent);
-#if defined(VMTOOLS_USE_GLIB)
-   g_source_unref(in->nextEvent);
-#endif
-   in->nextEvent = NULL;
+   current = in->delay;
 
    /* This is very important: this is the only way to signal the existence
       of this guest application to VMware */
@@ -743,8 +743,13 @@ RpcInLoop(void *clientData) // IN
    in->mustSend = TRUE;
 
 #if defined(VMTOOLS_USE_GLIB)
-   RPCIN_SCHED_EVENT(in, g_timeout_source_new(in->delay * 10));
+   resched = (in->delay != current);
+   if (resched) {
+      g_source_unref(in->nextEvent);
+      RPCIN_SCHED_EVENT(in, VMTools_CreateTimer(in->delay * 10));
+   }
 #else
+   resched = TRUE; /* Avoid unused warning. */
    in->nextEvent = EventManager_Add(gTimerEventQueue, in->delay, RpcInLoop, in);
 #endif
    if (in->nextEvent == NULL) {
@@ -753,7 +758,7 @@ RpcInLoop(void *clientData) // IN
    }
 
 #if defined(VMTOOLS_USE_GLIB)
-   return FALSE;
+   return !resched;
 #else
    return TRUE;
 #endif
@@ -827,7 +832,7 @@ RpcIn_start(RpcIn *in,                    // IN
 
    ASSERT(in->nextEvent == NULL);
 #if defined(VMTOOLS_USE_GLIB)
-   RPCIN_SCHED_EVENT(in, g_timeout_source_new(in->delay * 10));
+   RPCIN_SCHED_EVENT(in, VMTools_CreateTimer(in->delay * 10));
 #else
    in->nextEvent = EventManager_Add(gTimerEventQueue, 0, RpcInLoop, in);
    if (in->nextEvent == NULL) {
index f6ada3c37319aa5272d2f692ac1de727e499a3ee..1aab42ea2a3115d2a5cdf544f9d0ddabef4f0a58 100644 (file)
@@ -55,6 +55,7 @@ endif
 libvmtools_la_SOURCES =
 libvmtools_la_SOURCES += fileLogger.c
 libvmtools_la_SOURCES += i18n.c
+libvmtools_la_SOURCES += monotonicTimer.c
 libvmtools_la_SOURCES += signalSource.c
 libvmtools_la_SOURCES += stdLogger.c
 libvmtools_la_SOURCES += sysLogger.c
diff --git a/open-vm-tools/libvmtools/monotonicTimer.c b/open-vm-tools/libvmtools/monotonicTimer.c
new file mode 100644 (file)
index 0000000..eb2fb34
--- /dev/null
@@ -0,0 +1,191 @@
+/*********************************************************
+ * Copyright (C) 2008 VMware, Inc. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation version 2.1 and no later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the Lesser GNU General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA.
+ *
+ *********************************************************/
+
+/**
+ * @file monotonicTimer.c
+ *
+ * A GSource that implements a timer backed by a monotonic time source.
+ */
+
+#include <limits.h>
+#include "vmware.h"
+#include "system.h"
+#include "vmware/tools/utils.h"
+
+typedef struct MTimerSource {
+   GSource     src;
+   gint        timeout;
+   uint64      last;
+} MTimerSource;
+
+
+/*
+ *******************************************************************************
+ * MTimerSourcePrepare --                                                 */ /**
+ *
+ * Callback for the "prepare()" event source function. Sets the timeout to
+ * the number of milliseconds this timer expects to sleep for. If the timeout
+ * has already expired, update the internal state tracking the last time the
+ * timer was fired.
+ *
+ * @param[in]  src         The source.
+ * @param[out] timeout     Where to store the timeout.
+ *
+ * @return TRUE if timeout has already expired.
+ *
+ *******************************************************************************
+ */
+
+static gboolean
+MTimerSourcePrepare(GSource *src,
+                    gint *timeout)
+{
+   MTimerSource *timer = (MTimerSource *) src;
+
+   if (timer->timeout == 0) {
+      *timeout = 0;
+      return TRUE;
+   } else {
+         uint64 now = System_GetTimeMonotonic() * 10;
+         uint64 diff;
+
+         ASSERT(now >= timer->last);
+
+         diff = now - timer->last;
+         if (diff >= timer->timeout) {
+            timer->last = now;
+            *timeout = 0;
+            return TRUE;
+         }
+
+      *timeout = MIN(INT_MAX, timer->timeout - diff);
+      return FALSE;
+   }
+}
+
+
+/*
+ *******************************************************************************
+ * MTimerSourceCheck --                                                   */ /**
+ *
+ * Checks whether the timeout has expired.
+ *
+ * @param[in]  src     The source.
+ *
+ * @return Whether the timeout has expired.
+ *
+ *******************************************************************************
+ */
+
+static gboolean
+MTimerSourceCheck(GSource *src)
+{
+   gint unused;
+   return MTimerSourcePrepare(src, &unused);
+}
+
+
+/*
+ *******************************************************************************
+ * MTimerSourceDispatch --                                                */ /**
+ *
+ * Calls the callback associated with the timer, if any.
+ *
+ * @param[in]  src         Unused.
+ * @param[in]  callback    The callback to be called.
+ * @param[in]  data        User-supplied data.
+ *
+ * @return The return value of the callback, or FALSE if the callback is NULL.
+ *
+ *******************************************************************************
+ */
+
+static gboolean
+MTimerSourceDispatch(GSource *src,
+                     GSourceFunc callback,
+                     gpointer data)
+{
+   return (callback != NULL) ? callback(data) : FALSE;
+}
+
+
+/*
+ *******************************************************************************
+ * MTimerSourceFinalize --                                                */ /**
+ *
+ * Does nothing. The main glib code already does all the cleanup needed.
+ *
+ * @param[in]  src     The source.
+ *
+ *******************************************************************************
+ */
+
+static void
+MTimerSourceFinalize(GSource *src)
+{
+}
+
+
+/**
+ *
+ * @addtogroup vmtools_utils
+ * @{
+ */
+
+/*
+ *******************************************************************************
+ * VMTools_CreateTimer --                                                 */ /**
+ *
+ * @brief Create a timer based on a monotonic clock source.
+ *
+ * This timer differs from the glib timeout source, which uses the system time.
+ * It is recommended for code that needs more reliable time tracking, using a
+ * clock that is not affected by changes in the system time (which can happen
+ * when using NTP or the Tools time synchronization feature).
+ *
+ * @param[in] timeout   The timeout for the timer, must be >= 0.
+ *
+ * @return The new source.
+ *
+ *******************************************************************************
+ */
+
+GSource *
+VMTools_CreateTimer(gint timeout)
+{
+   static GSourceFuncs srcFuncs = {
+      MTimerSourcePrepare,
+      MTimerSourceCheck,
+      MTimerSourceDispatch,
+      MTimerSourceFinalize,
+      NULL,
+      NULL
+   };
+   MTimerSource *ret;
+
+   ASSERT(timeout >= 0);
+
+   ret = (MTimerSource *) g_source_new(&srcFuncs, sizeof *ret);
+   ret->last = System_GetTimeMonotonic() * 10;
+   ret->timeout = timeout;
+
+   return &ret->src;
+}
+
+/** @}  */
+
index 6a13a55cd3f79c81c6fa5bfdf14a5029686f4e80..582f33462e6bffc7e638e636a12728c1832f0f15 100644 (file)
@@ -489,7 +489,7 @@ CopyPasteDnDWrapper::OnReset()
    GSource *src;
 
    g_debug("%s: enter\n", __FUNCTION__);
-   src = g_timeout_source_new(RPC_POLL_TIME * 30);
+   src = VMTools_CreateTimer(RPC_POLL_TIME * 30);
    if (src) {
       VMTOOLSAPP_ATTACH_SOURCE(m_ctx, src, DnDPluginResetSent, this, NULL);
       g_source_unref(src);
index 58b1b7541fad809985ccc4589294420d37b9a8e5..8a4f99b25dd9e810da25d5f7c1675e118b53384d 100644 (file)
@@ -37,6 +37,7 @@ extern "C" {
 #include "copyPasteDnDWrapper.h"
 
 #include "pointer.h"
+#include "vmware/tools/utils.h"
 
 extern "C" {
    #include "vm_assert.h"
@@ -277,11 +278,9 @@ PointerUpdatePointerLoop(gpointer clientData) // IN: unused
       CopyPasteDnDWrapper *wrapper = CopyPasteDnDWrapper::GetInstance();
       ToolsAppCtx *ctx = wrapper->GetToolsAppCtx();
       if (ctx) {
-         src = g_timeout_source_new(POINTER_UPDATE_TIMEOUT);
-         if (src) {
-            VMTOOLSAPP_ATTACH_SOURCE(ctx, src, PointerUpdatePointerLoop, NULL, NULL);
-            g_source_unref(src);
-         }
+         src = VMTools_CreateTimer(POINTER_UPDATE_TIMEOUT);
+         VMTOOLSAPP_ATTACH_SOURCE(ctx, src, PointerUpdatePointerLoop, NULL, NULL);
+         g_source_unref(src);
       }
    }