[Spice-devel] [PATCH v2 2/3] tests: create and use a template file for events

[Frediano Ziglio]

> 
> On Thu, Dec 17, 2015 at 10:04:29AM -0500, Frediano Ziglio wrote:
> > > > +static void CORE_NAME(timer_start)(SpiceTimer *timer, uint32_t ms)
> > > > +{
> > > > +    CORE_NAME(timer_cancel)(timer);
> > > > +
> > > > +    timer->source = g_timeout_source_new(ms);
> > > > +    spice_assert(timer->source != NULL);
> > > > +
> > > > +    g_source_set_callback(timer->source, CORE_NAME(timer_func), timer,
> > > > NULL);
> > > > +
> > > > +    g_source_attach(timer->source, CORE_MAIN_CONTEXT(timer->opaque));
> > > > +}
> > > 
> > > It's used to pick the correct main context so that things run in the
> > > correct thread when this is used from a worker thread.
> > > However, this looks like what
> > > g_main_context_get_thread_default()/g_main_context_push_thread_default()
> > > are meant to address (see proposed patch below).
> > > 
> > 
> > No, this currently does not work.
> > The function is called also outside the worker thread (I start wondering
> > that
> > building displar and cursor channel inside the worker thread was right..
> > this was changed by one of first refactory patches).
> 
> Can you be more specific about the commit(s) you have in mind? From my
> reading of the code, before the switch to glib mainloop,
> timer_add/timer_start have to be called from the same thread, the
> comment in worker_get_main_context_from_opaque() would imply that
> the only timer we area interested in is the one setup in
> red_channel_client_create(), and this runs in the worker thread for
> the cursor channel as far as I can tell.
> I'm most likely missing something, since it's still fresh on your mind,
> better to ask ;)
> 
> Christophe
> 

I think I was confused by CursorChannelClient and DisplayChannelClient.

The CORE_MAIN_CONTEXT/event_loop_context_from_opaque are called inside
timer_add and watch_update_mask (which is called from watch_add).

red_channel_client_create (called by common_channel_new_client called by
cursor_channel_client_new and dcc_new). So these function should be called
inside the worker thread. And they are!

The patch is this anyway
http://cgit.freedesktop.org/~fziglio/spice-server/commit/?id=452edd8f7aa25fc1e69b6c2a747f59f58ab07f32

So could be your patch can work.
However the main idea of my patch was not to fix this "strange" code but
to have a single source for the event loop with better (and automated)
way of testing it.

And still I would prefer a simple __thread variable.

Frediano