GNOME Bugzilla – Bug 763168
gldisplay: _get_gl_context_for_thread_unlocked looks suspicious
Last modified: 2016-03-11 15:56:34 UTC
Created attachment 323187 [details] [review] gldisplay: really retrieve glcontext for a specific thread See attached patch for the minor change; which unless I am missing the point should make the function really do as intended (whereas it now might fail to retrieve desired context). Impact presently seems minor due to the limited code paths (and parameters) it is invoked.
Review of attachment 323187 [details] [review]: Still not quite right. ::: gst-libs/gst/gl/gstgldisplay.c @@ +440,2 @@ context_thread = gst_gl_context_get_thread (context); + if (thread != NULL && thread != context_thread) { Both before and after these changes aren't quite correct. Before is definitely wrong. After, if thread and all context_thread's are all NULL, then no context will be returned. This should probably be something like this instead: if (thread == NULL) return context; context_thread = gst_gl_context_get_thread (context); if (thread != context_thread) { ... Or invert the (thread != context_thread) and switch the continue/later return code paths
Created attachment 323189 [details] [review] gldisplay: really retrieve glcontext for a specific thread Oops, indeed. Modified patch along suggested lines ...
Review of attachment 323189 [details] [review]: Some explanation in the commit message on why this is needed would be nice :)
commit e38af2304427db908a16bbae0e60aa68be1ba5b5 Author: Mark Nauwelaerts <mnauw@users.sourceforge.net> Date: Sat Mar 5 17:16:24 2016 +0100 gldisplay: really retrieve glcontext for a specific thread When requesting a glcontext (regardless of thread), the result was correct. However, when requesting current glcontext on a specific thread, it could come up with a glcontext active on another thread. https://bugzilla.gnome.org/show_bug.cgi?id=763168