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

[Christophe Fergeau]

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.

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160323/0957f3de/attachment-0001.sig>