[Spice-devel] [spice v10 10/27] server: Make the RedDrawable refcount thread-safe

[Frediano Ziglio]

> 
> Hey,
> 
> On Wed, Mar 23, 2016 at 05:32:22AM -0400, Frediano Ziglio wrote:
> > >  - But while QXLState does have a reds field, it's defined in red-qxl.c
> > >    so it's not accessible from red-worker.c and I have not found a
> > >    function that would return this information.
> > 
> > You could use red_qxl_get_server but as said before you would get
> > the wrong context.
> > 
> > I think here what's missing is a way to get a RedWorker from a QXLInstance.
> > This could be added to red-qxl.[ch].
> 
> For what it's worth, the only thing which is needed is the GMainContext,
> not the full-fledged RedWorker. It seems going from RedWorker to
> QXLInstance is easy, but I did not see an obvious way of getting a
> RedWorker from a QXLInstance.
> I'm going to sound like a broken record, but adding
> 
> @@ -1551,14 +1568,34 @@ SPICE_GNUC_NORETURN static void *red_worker_main(void
> *arg)
>      RED_CHANNEL(worker->cursor_channel)->thread_id = pthread_self();
>      RED_CHANNEL(worker->display_channel)->thread_id = pthread_self();
> 
> +    g_main_context_push_thread_default(worker->core.main_context);
>      GMainLoop *loop = g_main_loop_new(worker->core.main_context, FALSE);
>      g_main_loop_run(loop);
>      g_main_loop_unref(loop);
> +    g_main_context_pop_thread_default(worker->core.main_context);
> 
>      /* FIXME: free worker, and join threads */
>      exit(0);
>  }
> 
> and using g_main_context_get_thread_default() at the right time (ie when we
> are
> in the display thread, not in one of the GStreamer threads) would do the
> trick as well.
> 

yes, but g_main_context_push_thread_default works if you are using the same
thread but if GStreamer spawn their own thread you still have to get the
context from there.

> Otherwise, I've tested that something like
> 
> +void red_worker_queue_idle(RedWorker *worker, GSourceFunc idle_callback,
> gpointer user_data)
> +{
> +    //GSource *source = g_timeout_source_new(1);
> +    GSource *source = g_idle_source_new();
> +
> +    g_source_set_callback(source, idle_callback, user_data, NULL);
> +    //g_source_set_priority (source, G_PRIORITY_HIGH_IDLE);
> +    g_source_attach(source, worker->core.main_context);
> +    g_source_unref(source);
> +}
> 
> works provided you have a RedWorker pointer. One gotcha though is that
> queueing an idle this way is currently not working, you have to
> workaround it with g_timeout_source_new(1).
> The reason for that is worker_source_check() which always returns TRUE.
> This means worker_source_dispatch() is going to run at every mainloop
> iteration, preventing idles from running as there is non-idle work to be
> done. We'll have to fix that too... :)
> 
> Christophe
> 

One workaround would be to set idle priority for this source but could cause
other troubles.

Frediano