From 583fb1a397023257ea3d2ee81f88b360007f07a2 Mon Sep 17 00:00:00 2001 From: Christoph Reiter Date: Thu, 10 May 2018 13:28:49 +0200 Subject: [PATCH] win32: make g_cond_wait_until() wait at least until end_time before returning with a timeout. See #1371 The tests in test_async_queue_timed() assume that g_async_queue_timeout_pop() and in turn g_cond_wait_until() wait at least until end_time before returning, i.e. calling g_get_monotonic_time() after the timeout should result in a value equal or larger than the timeout end time. For the win32 implementation of g_cond_wait_until() this isn't the case which makes those tests fail. There are three reasons why the function returns early: 1) The underlying API works with milliseconds and the timeout gets rounded down, resulting in a too small timeout value. 2) In case the timeout is too large to be passed to the API it gets limited (there is also a bug because it converts INFINITE to milliseconds while they already are, but using INFINITE would be wrong as well, as passing a large timeout is not the same as blocking forever really) 3) Even with the rounding changed the underlying API still returns a bit early sometimes on my machine (relative to g_get_monotonic_time()) This changes the implementation to round up to the next millisecond (fixing 1) and to wait again in case a timeout occurs but the end time hasn't been reached yet (fixing 2 and 3). This makes the test_async_queue_timed() tests pass. https://bugzilla.gnome.org/show_bug.cgi?id=795569 --- glib/gthread-win32.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index 938ed5f3c..a81f00a5c 100644 --- a/glib/gthread-win32.c +++ b/glib/gthread-win32.c @@ -305,17 +305,38 @@ g_cond_wait_until (GCond *cond, GMutex *entered_mutex, gint64 end_time) { - gint64 span; + gint64 span, start_time; + DWORD span_millis; + gboolean signalled; - span = end_time - g_get_monotonic_time (); + start_time = g_get_monotonic_time (); + do + { + span = end_time - start_time; - if G_UNLIKELY (span < 0) - span = 0; + if G_UNLIKELY (span < 0) + span_millis = 0; + else if G_UNLIKELY (span > G_GINT64_CONSTANT (1000) * (DWORD) INFINITE) + span_millis = INFINITE; + else + /* Round up so we don't time out too early */ + span_millis = (span + 1000 - 1) / 1000; - if G_UNLIKELY (span > G_GINT64_CONSTANT (1000) * G_MAXINT32) - span = INFINITE; + /* We never want to wait infinitely */ + if (span_millis >= INFINITE) + span_millis = INFINITE - 1; - return g_thread_impl_vtable.SleepConditionVariableSRW (cond, entered_mutex, span / 1000, 0); + signalled = g_thread_impl_vtable.SleepConditionVariableSRW (cond, entered_mutex, span_millis, 0); + if (signalled) + break; + + /* In case we didn't wait long enough after a timeout, wait again for the + * remaining time */ + start_time = g_get_monotonic_time (); + } + while (start_time < end_time); + + return signalled; } /* {{{1 GPrivate */