After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 775171 - glcontext/glwindow: Race when creating/quitting
glcontext/glwindow: Race when creating/quitting
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.11.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-11-27 12:02 UTC by Edward Hervey
Modified: 2016-11-28 03:28 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Edward Hervey 2016-11-27 12:02:18 UTC
This is the core issue exposed by the various hanging gl unit tests

The "easiest" to trigger the issue is test_is_shared.

The problem is the following:
1) A context is created
2) It creates/starts a thread
  2.1) That thread asks for _unlock_create_thread() to be called
  2.2) Then calls gst_gl_window_run which will eventually start a mainloop
3) In the meantime the context is finalized
 3.1) We call gst_gl_window_quit which will stop the mainloop ...
   ... but that mainloop hasn't started yet !
 3.2) It unlocks the render_lock and joins the thread
 2.3) The other thread finally calls main_loop_run ...

 ... hang
   
Something is needed to avoid that race, but i'm not 100% sure yet.
Comment 1 Matthew Waters (ystreet00) 2016-11-28 01:23:34 UTC
This is due to 626bcccff96f624f59c5212b3e21e472240171fd removing locks on shutdown and the previous fix 2776cef25d2a98668b73272aecfe77e684e6627e not adding them back correctly.
Comment 2 Matthew Waters (ystreet00) 2016-11-28 01:27:36 UTC
The other failure case in the new code with g_thread_join() is if the last ref is held by the GL thread.  Then g_thread_join() that will fail.
Comment 3 Matthew Waters (ystreet00) 2016-11-28 02:33:10 UTC
The second problem is that the g_main_context_push_thread_default() in 072183ce616760c8859d5c14f3a440be003891bf makes the g_main_context_invoke() in gst_gl_window_send_message_async() run the function immediately rather than deferring it until later as is required to have the loop running before returning from gst_gl_context_create().
Comment 4 Matthew Waters (ystreet00) 2016-11-28 03:28:49 UTC
commit 024e92afe7bb6ab019876ceeb97041129b95c0d7
Author: Matthew Waters <matthew@centricular.com>
Date:   Mon Nov 28 14:22:05 2016 +1100

    glwindow: move g_main_context_push/pop_thread_default() to run()
    
    Calling g_main_context_push_thread and then g_main_context_invoke()
    (used by gst_gl_window_send_message_async()) in the same thread will
    cause the invoked function to run immediately instead of being delayed.
    
    This had implications for the creation of the OpenGL context not waiting
    until the main loop had completely started up and as a result would
    sometimes deadlock in short create/destroy scenarios.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=775171

commit 25fbc6d877b81fb5cd2731edc01d05bf900420dc
Author: Matthew Waters <matthew@centricular.com>
Date:   Mon Nov 28 14:19:18 2016 +1100

    glcontext: fix race between creation/shutdown
    
    626bcccff96f624f59c5212b3e21e472240171fd removed some locks that
    allowed the main loop quit to occur before the context was fully
    created.
    
    2776cef25d2a98668b73272aecfe77e684e6627e attempted to readd them but
    missed the scop of the quit() call.
    
    Also remove the use of g_thread_join() as that's not safe to use when
    it's possible to lose the last reference from the GL thread.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=775171