[Spice-commits] 2 commits - server/event-loop.c server/tests

[Frediano Ziglio]

 server/event-loop.c      |   51 -----------------------------------------------
 server/tests/test-loop.c |   40 +++++++++++++++++++++++++++---------
 2 files changed, 30 insertions(+), 61 deletions(-)

New commits:
commit 47223da1a3d5762b7d7bb5e06d3a32aa701832d8
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Fri Feb 12 17:13:44 2016 +0000

    revert new event-loop code for timers
    
    Was causing sporadic crashes.
    Also cause compatibility problems with RHEL 6.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/event-loop.c b/server/event-loop.c
index 9eae913..4738ed9 100644
--- a/server/event-loop.c
+++ b/server/event-loop.c
@@ -24,56 +24,6 @@
 
 #include "red-common.h"
 
-#if GLIB_CHECK_VERSION(2, 34, 0)
-struct SpiceTimer {
-    GSource source;
-};
-
-static gboolean
-spice_timer_dispatch(GSource     *source,
-                     GSourceFunc  callback,
-                     gpointer     user_data)
-{
-    SpiceTimerFunc func = (SpiceTimerFunc) callback;
-
-    func(user_data);
-    /* timer might be free after func(), don't touch */
-
-    return FALSE;
-}
-
-static GSourceFuncs spice_timer_funcs = {
-    .dispatch = spice_timer_dispatch,
-};
-
-static SpiceTimer* timer_add(const SpiceCoreInterfaceInternal *iface,
-                             SpiceTimerFunc func, void *opaque)
-{
-    SpiceTimer *timer = (SpiceTimer *) g_source_new(&spice_timer_funcs, sizeof(SpiceTimer));
-
-    g_source_set_callback(&timer->source, (GSourceFunc) func, opaque, NULL);
-
-    g_source_attach(&timer->source, iface->main_context);
-
-    return timer;
-}
-
-static void timer_cancel(SpiceTimer *timer)
-{
-    g_source_set_ready_time(&timer->source, -1);
-}
-
-static void timer_start(SpiceTimer *timer, uint32_t ms)
-{
-    g_source_set_ready_time(&timer->source, g_get_monotonic_time() + ms * 1000u);
-}
-
-static void timer_remove(SpiceTimer *timer)
-{
-    g_source_destroy(&timer->source);
-    g_source_unref(&timer->source);
-}
-#else
 struct SpiceTimer {
     GMainContext *context;
     SpiceTimerFunc func;
@@ -130,7 +80,6 @@ static void timer_remove(SpiceTimer *timer)
     spice_assert(timer->source == NULL);
     free(timer);
 }
-#endif
 
 struct SpiceWatch {
     GMainContext *context;
commit 6e43433ec29594dccbd1b6d095c0ac87ece75bbc
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Feb 4 16:00:26 2016 +0000

    tests: add a new case for event loop timers
    
    Check that cancelling a timer the timer callback is not called.
    
    This can happen in latency code (red-channel.c).
    In red_channel_client_cancel_ping_timer latency timer is cancelled and
    state is set to PING_STATE_NONE however if timer was already active what
    happens is that the red_channel_client_ping_timer is called and the line
    
      spice_assert(rcc->latency_monitor.state == PING_STATE_TIMER);
    
    is triggered causing spice-server to abort.
    This happens as GLib loop add all active sources to an array but if the
    timer is deactivated before the event is dispatched the event will be
    dispatched unless the source is destroyed.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>

diff --git a/server/tests/test-loop.c b/server/tests/test-loop.c
index 1c2f496..0bcc953 100644
--- a/server/tests/test-loop.c
+++ b/server/tests/test-loop.c
@@ -74,19 +74,31 @@ static void *loop_func(void *arg)
     return NULL;
 }
 
-static SpiceTimer *twice_timers[2] = { NULL, NULL };
-static int twice_called = 0;
-static void timer_not_twice(void *opaque)
+static SpiceTimer *twice_timers_remove[2] = { NULL, NULL };
+static int twice_remove_called = 0;
+static void timer_not_twice_remove(void *opaque)
 {
-    spice_assert(++twice_called == 1);
+    spice_assert(++twice_remove_called == 1);
 
     /* delete timers, should not have another call */
-    core->timer_remove(twice_timers[0]);
-    core->timer_remove(twice_timers[1]);
-    twice_timers[0] = NULL;
-    twice_timers[1] = NULL;
+    core->timer_remove(twice_timers_remove[0]);
+    core->timer_remove(twice_timers_remove[1]);
+    twice_timers_remove[0] = NULL;
+    twice_timers_remove[1] = NULL;
+}
+
+static SpiceTimer *twice_timers_cancel[2] = { NULL, NULL };
+static int twice_cancel_called = 0;
+static void timer_not_twice(void *opaque)
+{
+    spice_assert(++twice_cancel_called == 1);
+
+    /* cancel timers, should not have another call */
+    core->timer_cancel(twice_timers_cancel[0]);
+    core->timer_cancel(twice_timers_cancel[1]);
 }
 
+
 int main(int argc, char **argv)
 {
     SpiceTimer *timer, *timers[10];
@@ -121,13 +133,21 @@ int main(int argc, char **argv)
     core->timer_start(timer, 10);
 
     /* test events are not called when freed */
-    timer = twice_timers[0] = core->timer_add(timer_not_twice, NULL);
+    timer = twice_timers_remove[0] = core->timer_add(timer_not_twice_remove, NULL);
     spice_assert(timer != NULL);
     core->timer_start(timer, 2);
-    timer = twice_timers[1] = core->timer_add(timer_not_twice, NULL);
+    timer = twice_timers_remove[1] = core->timer_add(timer_not_twice_remove, NULL);
     spice_assert(timer != NULL);
     core->timer_start(timer, 2);
 
+    /* test events are not called when cancelled */
+    timer = timers[i++] = twice_timers_cancel[0] = core->timer_add(timer_not_twice, core);
+    spice_assert(timer != NULL);
+    core->timer_start(timer, 4);
+    timer = timers[i++] = twice_timers_cancel[1] = core->timer_add(timer_not_twice, core);
+    spice_assert(timer != NULL);
+    core->timer_start(timer, 4);
+
     /* run the loop */
     loop = g_main_loop_new(basic_event_loop_get_context(), FALSE);
     alarm(1);