GNOME Bugzilla – Bug 779181
gst_gl_window_eagl_new can be called multiple times in different threads, which can lead to deadlocks
Last modified: 2017-02-26 11:20:27 UTC
Created attachment 346642 [details] Stacktraces It says that /* Must be called in the gl thread */ GstGLWindowEagl * gst_gl_window_eagl_new (GstGLDisplay * display) However, it runs in the main thread if "gst_gl_basefilter_decide_allocation / ... / gst_gl_context_create / ... / _ensure_window" is called Or if "gst_glimage_sink_change_state / _ensure_gl_setup ... / _ensure_window" is called. And, in fact, these two things could run in multiple threads if queues and bins are involved and things are set in state playing before being linked together. This will lead to running this multiple times: gl_queue = dispatch_queue_create ("org.freedesktop.gstreamer.glwindow", NULL); Which is bad, because when the elements are linked together the program will deadlock. I've attached a picture showing a deadlock. Note that their are two threads with the same name "org.freedesktop.gstreamer.glwindow", which I think should never happen. I think the fix is to EITHER make sure the window really is only called in the gl thread OR to make it use a singleton/static variable that is only initialized once.
(In reply to Nick Kallen from comment #0) > Created attachment 346642 [details] > Stacktraces > > It says that > > /* Must be called in the gl thread */ > GstGLWindowEagl * > gst_gl_window_eagl_new (GstGLDisplay * display) > > However, it runs in the main thread if "gst_gl_basefilter_decide_allocation > / ... / gst_gl_context_create / ... / _ensure_window" is called > > Or if "gst_glimage_sink_change_state / _ensure_gl_setup ... / > _ensure_window" is called. The comment could be old information, and honestly it probably is. > And, in fact, these two things could run in multiple threads if queues and > bins are involved and things are set in state playing before being linked > together. This sounds like a very bad idea without dealing with the ramifications like this problem of having multiple GL context in the pipeline. Also, do you deal with the not-linked flow returns? Negotiation? This doesn't make sense. There are fallbacks for the cases where two elements who want to share GL context's aren't linked together but that requires them to be in the same pipeline. What's your pipeline? Do you have a test case that you can attach? Preferably reproducible on desktop as well. > This will lead to running this multiple times: > > gl_queue = dispatch_queue_create ("org.freedesktop.gstreamer.glwindow", > NULL); > > Which is bad, because when the elements are linked together the program will > deadlock. I've attached a picture showing a deadlock. Note that their are > two threads with the same name "org.freedesktop.gstreamer.glwindow", which I > think should never happen. There will be one dispatch queue per GL context created as the GL context needs a GL thread provided by the window and to avoid context switching between multiple GL contexts, they need their own threads. > I think the fix is to EITHER make sure the window really is only called in > the gl thread OR to make it use a singleton/static variable that is only > initialized once. Singleton is not possible due to above. What do you mean by "window really is only called in the gl thread"?
My pipeline is complicated because it changes dynamically in response to user input. I use async bins and in general I set things to playing before I add them to the pipeline because it seems to create noticable performance improvements. This works well in general. However, if I used the same technique a startup and there is a gl-filter based source, I got this deadlock. Here is the simplest I can reproduce process start: 1. create pipeline 2. set state playing wait for window to be available: 3. create async bin "videotestsrc ! glupload" 4. set state playing 5. create async bin " .... ! glimagesink" 6. set window handle 7. set state playing 6. add both to pipeline then it deadlocks. In this case, the two async bins create their own gl contexts and their own dispatch queues. Then when they are linked together, they still have their own dispatch queues, which leads to the deadlock above.
I doubt this would be reproducible on desktop since the cocoa code seems to use a singleton.
The cocoa code doesn't use a singleton for this part, none of the implementations do. (In reply to Nick Kallen from comment #2) > My pipeline is complicated because it changes dynamically in response to > user input. I use async bins and in general I set things to playing before I > add them to the pipeline because it seems to create noticable performance > improvements. This works well in general. > > However, if I used the same technique a startup and there is a gl-filter > based source, I got this deadlock. > > Here is the simplest I can reproduce > > process start: > > 1. create pipeline > 2. set state playing If you do this, you need to either 1. Proxy the context queries between elements 2. Have all the GL elements in the same pipeline before READY->PAUSED (or NULL->READY for video sinks). > wait for window to be available: What does this mean? > 3. create async bin "videotestsrc ! glupload" > 4. set state playing Side note, how do you deal with not-linked if glupload isn't linked to anything? > 5. create async bin " .... ! glimagesink" > 6. set window handle > 7. set state playing > 6. add both to pipeline > > then it deadlocks. > > In this case, the two async bins create their own gl contexts and their own > dispatch queues. Then when they are linked together, they still have their > own dispatch queues, which leads to the deadlock above. That is definitely not surprising and completely unsupported. You need to have glimagesink create the GL context as it is the element that is displaying the stream and needs to perform winsys specific operations for that to happen. To perform what you're attempting by having a GL context being replaced by another is a complicated endeavour.
I 80% understand what you're saying. Would it be easy/possible for it to give a nice warning rather than deadlock? > Side note, how do you deal with not-linked if glupload isn't linked to anything? I think this is what async bins do for you. They can do some of the setup and caps negotiation but not all. When I wasn't using async bins setting the bins to the playing state when they were in the pipeline would sometimes pause the entire pipeline.
(In reply to Nick Kallen from comment #5) > I 80% understand what you're saying. Would it be easy/possible for it to > give a nice warning rather than deadlock? Not really, I can't really think of a way to detect this that doesn't also produce false positives. Unless somebody wants to go through the procedure of allowing GL contexts/resources to be replaced at runtime, I'm going to mark this a WONTFIX.