[Spice-devel] [qxl] xspice: Don't create Xorg time in timer_add

[Jonathon Jongsma]

Ah, for some reason I thought this was reviewed already.

Looks like a good idea to me

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>


On Tue, 2016-04-12 at 15:20 +0200, Christophe Fergeau wrote:
> Ping?
> 
> Christophe
> 
> On Wed, Apr 06, 2016 at 01:35:04PM +0200, Christophe Fergeau wrote:
> > SpiceCoreInterface::timer_add() is used by spice-server for integration
> > with external mainloops. timer_add() is only meant to create a disabled
> > timer, this timer will then be started with a call to timer_start().
> > 
> > The current implementation in Xspice creates a timer which will trigger
> > in a very long time, assuming this will never happen. This 'forever' is
> > 1,000,000 seconds, which amounts to 11 days. After that time, some
> > timers which are meant to be disabled (eg migration related timers in
> > spice-server) fire, then causing a crash with some failed assertions.
> > 
> > Instead of creating the X timer right away in timer_add(), we can wait
> > until timer_start() is called before starting it, which avoids this
> > issue.
> > ---
> >  src/spiceqxl_main_loop.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/spiceqxl_main_loop.c b/src/spiceqxl_main_loop.c
> > index ac9e43f..db89b6d 100644
> > --- a/src/spiceqxl_main_loop.c
> > +++ b/src/spiceqxl_main_loop.c
> > @@ -171,7 +171,6 @@ static SpiceTimer* timer_add(SpiceTimerFunc func, void
> > *opaque)
> >  {
> >      SpiceTimer *timer = calloc(sizeof(SpiceTimer), 1);
> >  
> > -    timer->xorg_timer = TimerSet(NULL, 0, 1e9 /* TODO: infinity? */,
> > xorg_timer_callback, timer);
> >      timer->func = func;
> >      timer->opaque = opaque;
> >      return timer;
> > @@ -179,7 +178,8 @@ static SpiceTimer* timer_add(SpiceTimerFunc func, void
> > *opaque)
> >  
> >  static void timer_start(SpiceTimer *timer, uint32_t ms)
> >  {
> > -    TimerSet(timer->xorg_timer, 0 /* flags */, ms, xorg_timer_callback,
> > timer);
> > +    timer->xorg_timer = TimerSet(timer->xorg_timer, 0 /* flags */,
> > +                                 ms, xorg_timer_callback, timer);
> >  }
> >  
> >  static void timer_cancel(SpiceTimer *timer)
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel