gl: avoid deadlock querying for OpenGL context

If there are two elements and threads attempting to query each other for
an OpenGL context. The locking may result in a deadlock.

We need to unlock each element's context_lock when querying another
element for the OpenGL context in order to allow any other element to
take the lock when the other element is querying for an OpenGL context.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/642>
This commit is contained in:
Matthew Waters 2020-04-24 14:44:46 +10:00
parent 4a7a247293
commit a4e49ba8c9
7 changed files with 449 additions and 70 deletions

View file

@ -99,13 +99,35 @@ gst_gl_base_mixer_pad_set_property (GObject * object, guint prop_id,
}
}
static gboolean
_find_local_gl_context (GstGLBaseMixer * mix)
_find_local_gl_context_unlocked (GstGLBaseMixer * mix)
{
GstGLContext *context = mix->context;
GstGLContext *context, *prev_context;
gboolean ret;
if (mix->context && mix->context->display == mix->display)
return TRUE;
context = prev_context = mix->context;
g_rec_mutex_unlock (&mix->priv->context_lock);
/* we need to drop the lock to query as another element may also be
* performing a context query on us which would also attempt to take the
* context_lock. Our query could block on the same lock in the other element.
*/
ret =
gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SRC, &context);
g_rec_mutex_lock (&mix->priv->context_lock);
if (ret) {
if (mix->context != prev_context) {
/* we need to recheck everything since we dropped the lock and the
* context has changed */
if (mix->context && mix->context->display == mix->display) {
if (context != mix->context)
gst_clear_object (&context);
return TRUE;
}
}
if (gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SRC, &context)) {
if (context->display == mix->display) {
mix->context = context;
return TRUE;
@ -113,7 +135,26 @@ _find_local_gl_context (GstGLBaseMixer * mix)
if (context != mix->context)
gst_clear_object (&context);
}
if (gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SINK, &context)) {
context = prev_context = mix->context;
g_rec_mutex_unlock (&mix->priv->context_lock);
/* we need to drop the lock to query as another element may also be
* performing a context query on us which would also attempt to take the
* context_lock. Our query could block on the same lock in the other element.
*/
ret =
gst_gl_query_local_gl_context (GST_ELEMENT (mix), GST_PAD_SINK, &context);
g_rec_mutex_lock (&mix->priv->context_lock);
if (ret) {
if (mix->context != prev_context) {
/* we need to recheck everything now that we dropped the lock */
if (mix->context && mix->context->display == mix->display) {
if (context != mix->context)
gst_clear_object (&context);
return TRUE;
}
}
if (context->display == mix->display) {
mix->context = context;
return TRUE;
@ -121,6 +162,7 @@ _find_local_gl_context (GstGLBaseMixer * mix)
if (context != mix->context)
gst_clear_object (&context);
}
return FALSE;
}
@ -140,7 +182,7 @@ _get_gl_context_unlocked (GstGLBaseMixer * mix)
gst_gl_display_filter_gl_api (mix->display, mix_class->supported_gl_api);
_find_local_gl_context (mix);
_find_local_gl_context_unlocked (mix);
GST_OBJECT_LOCK (mix->display);
if (!mix->context) {
@ -243,11 +285,26 @@ gst_gl_base_mixer_sink_query (GstAggregator * agg, GstAggregatorPad * bpad,
switch (GST_QUERY_TYPE (query)) {
case GST_QUERY_CONTEXT:
{
GstGLDisplay *display = NULL;
GstGLContext *other = NULL, *local = NULL;
gboolean ret;
g_rec_mutex_lock (&mix->priv->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) mix, query,
mix->display, mix->context, mix->priv->other_context);
if (mix->display)
display = gst_object_ref (mix->display);
if (mix->context)
local = gst_object_ref (mix->context);
if (mix->priv->other_context)
local = gst_object_ref (mix->priv->other_context);
g_rec_mutex_unlock (&mix->priv->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) mix, query,
display, local, other);
gst_clear_object (&display);
gst_clear_object (&other);
gst_clear_object (&local);
if (ret)
return ret;
break;
@ -438,7 +495,7 @@ gst_gl_base_mixer_activate (GstGLBaseMixer * mix, gboolean active)
g_rec_mutex_lock (&mix->priv->context_lock);
if (!gst_gl_ensure_element_data (mix, &mix->display,
&mix->priv->other_context)) {
g_rec_mutex_lock (&mix->priv->context_lock);
g_rec_mutex_unlock (&mix->priv->context_lock);
return FALSE;
}
@ -478,11 +535,26 @@ gst_gl_base_mixer_src_query (GstAggregator * agg, GstQuery * query)
switch (GST_QUERY_TYPE (query)) {
case GST_QUERY_CONTEXT:
{
GstGLDisplay *display = NULL;
GstGLContext *other = NULL, *local = NULL;
gboolean ret;
g_rec_mutex_lock (&mix->priv->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) mix, query,
mix->display, mix->context, mix->priv->other_context);
if (mix->display)
display = gst_object_ref (mix->display);
if (mix->context)
local = gst_object_ref (mix->context);
if (mix->priv->other_context)
local = gst_object_ref (mix->priv->other_context);
g_rec_mutex_unlock (&mix->priv->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) mix, query,
display, local, other);
gst_clear_object (&display);
gst_clear_object (&other);
gst_clear_object (&local);
if (ret)
return ret;
break;

View file

@ -121,6 +121,7 @@ enum
PROP_LATENCY,
PROP_START_TIME_SELECTION,
PROP_START_TIME,
PROP_CONTEXT,
};
enum
@ -204,6 +205,12 @@ gst_gl_mixer_bin_class_init (GstGLMixerBinClass * klass)
G_MAXUINT64,
DEFAULT_START_TIME, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
g_object_class_install_property (gobject_class, PROP_CONTEXT,
g_param_spec_object ("context",
"OpenGL context",
"Get OpenGL context",
GST_TYPE_GL_CONTEXT, G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
/**
* GstMixerBin::create-element:
* @object: the #GstGLMixerBin

View file

@ -411,6 +411,9 @@ stereosplit_set_output_caps (GstGLStereoSplit * split, GstCaps * sinkcaps)
goto fail;
}
/* FIXME: Provide left and right caps to do_bufferpool */
stereosplit_do_bufferpool (split, left);
g_rec_mutex_lock (&split->context_lock);
gst_gl_view_convert_set_context (split->viewconvert, split->context);
@ -424,9 +427,6 @@ stereosplit_set_output_caps (GstGLStereoSplit * split, GstCaps * sinkcaps)
GST_ERROR_OBJECT (split, "Failed to set caps on converter");
goto fail;
}
/* FIXME: Provide left and right caps to do_bufferpool */
stereosplit_do_bufferpool (split, left);
g_rec_mutex_unlock (&split->context_lock);
res = TRUE;
@ -444,10 +444,33 @@ fail:
static gboolean
_find_local_gl_context_unlocked (GstGLStereoSplit * split)
{
GstGLContext *context = split->context;
GstGLContext *context, *prev_context;
gboolean ret;
if (split->context && split->context->display == split->display)
return TRUE;
context = prev_context = split->context;
g_rec_mutex_unlock (&split->context_lock);
/* we need to drop the lock to query as another element may also be
* performing a context query on us which would also attempt to take the
* context_lock. Our query could block on the same lock in the other element.
*/
ret =
gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SRC,
&context);
g_rec_mutex_lock (&split->context_lock);
if (ret) {
if (split->context != prev_context) {
/* we need to recheck everything since we dropped the lock and the
* context has changed */
if (split->context && split->context->display == split->display) {
if (context != split->context)
gst_clear_object (&context);
return TRUE;
}
}
if (gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SRC,
&context)) {
if (context->display == split->display) {
split->context = context;
return TRUE;
@ -455,9 +478,27 @@ _find_local_gl_context_unlocked (GstGLStereoSplit * split)
if (context != split->context)
gst_clear_object (&context);
}
context = split->context;
if (gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SINK,
&context)) {
context = prev_context = split->context;
g_rec_mutex_unlock (&split->context_lock);
/* we need to drop the lock to query as another element may also be
* performing a context query on us which would also attempt to take the
* context_lock. Our query could block on the same lock in the other element.
*/
ret =
gst_gl_query_local_gl_context (GST_ELEMENT (split), GST_PAD_SINK,
&context);
g_rec_mutex_lock (&split->context_lock);
if (ret) {
if (split->context != prev_context) {
/* we need to recheck everything now that we dropped the lock */
if (split->context && split->context->display == split->display) {
if (context != split->context)
gst_clear_object (&context);
return TRUE;
}
}
if (context->display == split->display) {
split->context = context;
return TRUE;
@ -465,6 +506,7 @@ _find_local_gl_context_unlocked (GstGLStereoSplit * split)
if (context != split->context)
gst_clear_object (&context);
}
return FALSE;
}
@ -552,7 +594,6 @@ stereosplit_decide_allocation (GstGLStereoSplit * self, GstQuery * query)
return FALSE;
return TRUE;
}
static gboolean
@ -670,8 +711,26 @@ stereosplit_src_query (GstPad * pad, GstObject * parent, GstQuery * query)
switch (GST_QUERY_TYPE (query)) {
case GST_QUERY_CONTEXT:
{
if (gst_gl_handle_context_query ((GstElement *) split, query,
split->display, split->context, split->other_context))
GstGLDisplay *display = NULL;
GstGLContext *other = NULL, *local = NULL;
gboolean ret;
g_rec_mutex_lock (&split->context_lock);
if (split->display)
display = gst_object_ref (split->display);
if (split->context)
local = gst_object_ref (split->context);
if (split->other_context)
local = gst_object_ref (split->other_context);
g_rec_mutex_unlock (&split->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) split, query,
display, local, other);
gst_clear_object (&display);
gst_clear_object (&other);
gst_clear_object (&local);
if (ret)
return TRUE;
return gst_pad_query_default (pad, parent, query);
@ -699,8 +758,26 @@ stereosplit_sink_query (GstPad * pad, GstObject * parent, GstQuery * query)
switch (GST_QUERY_TYPE (query)) {
case GST_QUERY_CONTEXT:
{
if (gst_gl_handle_context_query ((GstElement *) split, query,
split->display, split->context, split->other_context))
GstGLDisplay *display = NULL;
GstGLContext *other = NULL, *local = NULL;
gboolean ret;
g_rec_mutex_lock (&split->context_lock);
if (split->display)
display = gst_object_ref (split->display);
if (split->context)
local = gst_object_ref (split->context);
if (split->other_context)
local = gst_object_ref (split->other_context);
g_rec_mutex_unlock (&split->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) split, query,
display, local, other);
gst_clear_object (&display);
gst_clear_object (&other);
gst_clear_object (&local);
if (ret)
return TRUE;
return gst_pad_query_default (pad, parent, query);

View file

@ -180,31 +180,83 @@ gst_gl_base_filter_get_property (GObject * object, guint prop_id,
}
}
static gboolean
_find_local_gl_context_unlocked (GstGLBaseFilter * filter)
{
GstGLContext *context, *prev_context;
gboolean ret;
if (filter->context && filter->context->display == filter->display)
return TRUE;
context = prev_context = filter->context;
g_rec_mutex_unlock (&filter->priv->context_lock);
/* we need to drop the lock to query as another element may also be
* performing a context query on us which would also attempt to take the
* context_lock. Our query could block on the same lock in the other element.
*/
ret =
gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SRC,
&context);
g_rec_mutex_lock (&filter->priv->context_lock);
if (ret) {
if (filter->context != prev_context) {
/* we need to recheck everything since we dropped the lock and the
* context has changed */
if (filter->context && filter->context->display == filter->display) {
if (context != filter->context)
gst_clear_object (&context);
return TRUE;
}
}
if (context->display == filter->display) {
filter->context = context;
return TRUE;
}
if (context != filter->context)
gst_clear_object (&context);
}
context = prev_context = filter->context;
g_rec_mutex_unlock (&filter->priv->context_lock);
/* we need to drop the lock to query as another element may also be
* performing a context query on us which would also attempt to take the
* context_lock. Our query could block on the same lock in the other element.
*/
ret =
gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SINK,
&context);
g_rec_mutex_lock (&filter->priv->context_lock);
if (ret) {
if (filter->context != prev_context) {
/* we need to recheck everything now that we dropped the lock */
if (filter->context && filter->context->display == filter->display) {
if (context != filter->context)
gst_clear_object (&context);
return TRUE;
}
}
if (context->display == filter->display) {
filter->context = context;
return TRUE;
}
if (context != filter->context)
gst_clear_object (&context);
}
return FALSE;
}
static gboolean
_find_local_gl_context (GstGLBaseFilter * filter)
{
GstGLContext *context = filter->context;
if (gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SRC,
&context)) {
if (context->display == filter->display) {
filter->context = context;
return TRUE;
}
if (context != filter->context)
gst_clear_object (&context);
}
context = filter->context;
if (gst_gl_query_local_gl_context (GST_ELEMENT (filter), GST_PAD_SINK,
&context)) {
if (context->display == filter->display) {
filter->context = context;
return TRUE;
}
if (context != filter->context)
gst_clear_object (&context);
}
return FALSE;
gboolean ret;
g_rec_mutex_lock (&filter->priv->context_lock);
ret = _find_local_gl_context_unlocked (filter);
g_rec_mutex_unlock (&filter->priv->context_lock);
return ret;
}
static gboolean
@ -218,9 +270,7 @@ gst_gl_base_filter_query (GstBaseTransform * trans, GstPadDirection direction,
{
if (direction == GST_PAD_SINK
&& gst_base_transform_is_passthrough (trans)) {
g_rec_mutex_lock (&filter->priv->context_lock);
_find_local_gl_context (filter);
g_rec_mutex_unlock (&filter->priv->context_lock);
return gst_pad_peer_query (GST_BASE_TRANSFORM_SRC_PAD (trans), query);
}
@ -228,11 +278,26 @@ gst_gl_base_filter_query (GstBaseTransform * trans, GstPadDirection direction,
}
case GST_QUERY_CONTEXT:
{
GstGLDisplay *display = NULL;
GstGLContext *other = NULL, *local = NULL;
gboolean ret;
g_rec_mutex_lock (&filter->priv->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) filter, query,
filter->display, filter->context, filter->priv->other_context);
if (filter->display)
display = gst_object_ref (filter->display);
if (filter->context)
local = gst_object_ref (filter->context);
if (filter->priv->other_context)
local = gst_object_ref (filter->priv->other_context);
g_rec_mutex_unlock (&filter->priv->context_lock);
ret = gst_gl_handle_context_query ((GstElement *) filter, query,
display, local, other);
gst_clear_object (&display);
gst_clear_object (&other);
gst_clear_object (&local);
if (ret)
return TRUE;
break;
@ -418,20 +483,9 @@ gst_gl_base_filter_change_state (GstElement * element,
switch (transition) {
case GST_STATE_CHANGE_READY_TO_NULL:
g_rec_mutex_lock (&filter->priv->context_lock);
if (filter->priv->other_context) {
gst_object_unref (filter->priv->other_context);
filter->priv->other_context = NULL;
}
if (filter->display) {
gst_object_unref (filter->display);
filter->display = NULL;
}
if (filter->context) {
gst_object_unref (filter->context);
filter->context = NULL;
}
gst_clear_object (&filter->priv->other_context);
gst_clear_object (&filter->display);
gst_clear_object (&filter->context);
g_rec_mutex_unlock (&filter->priv->context_lock);
break;
default:
@ -487,7 +541,7 @@ gst_gl_base_filter_find_gl_context_unlocked (GstGLBaseFilter * filter)
if (!filter->context)
new_context = TRUE;
_find_local_gl_context (filter);
_find_local_gl_context_unlocked (filter);
if (!filter->context) {
GST_OBJECT_LOCK (filter->display);

View file

@ -479,11 +479,34 @@ gst_gl_base_src_stop (GstBaseSrc * basesrc)
}
static gboolean
_find_local_gl_context (GstGLBaseSrc * src)
_find_local_gl_context_unlocked (GstGLBaseSrc * src)
{
GstGLContext *context = src->context;
GstGLContext *context, *prev_context;
gboolean ret;
if (src->context && src->context->display == src->display)
return TRUE;
context = prev_context = src->context;
g_rec_mutex_unlock (&src->priv->context_lock);
/* we need to drop the lock to query as another element may also be
* performing a context query on us which would also attempt to take the
* context_lock. Our query could block on the same lock in the other element.
*/
ret =
gst_gl_query_local_gl_context (GST_ELEMENT (src), GST_PAD_SRC, &context);
g_rec_mutex_lock (&src->priv->context_lock);
if (ret) {
if (src->context != prev_context) {
/* we need to recheck everything since we dropped the lock and the
* context has changed */
if (src->context && src->context->display == src->display) {
if (context != src->context)
gst_clear_object (&context);
return TRUE;
}
}
if (gst_gl_query_local_gl_context (GST_ELEMENT (src), GST_PAD_SRC, &context)) {
if (context->display == src->display) {
src->context = context;
return TRUE;
@ -513,7 +536,7 @@ gst_gl_base_src_find_gl_context_unlocked (GstGLBaseSrc * src)
gst_gl_display_filter_gl_api (src->display, klass->supported_gl_api);
_find_local_gl_context (src);
_find_local_gl_context_unlocked (src);
if (!src->context) {
GST_OBJECT_LOCK (src->display);

View file

@ -0,0 +1,145 @@
/* GStreamer
*
* Copyright (C) 2020 Matthew Waters <matthew@centricular.com>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
* License as published by the Free Software Foundation; either
* version 2 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Library General Public License for more details.
*
* You should have received a copy of the GNU Library General Public
* License along with this library; if not, write to the
* Free Software Foundation, Inc., 51 Franklin St, Fifth Floor,
* Boston, MA 02110-1301, USA.
*/
#ifdef HAVE_CONFIG_H
#include "config.h"
#endif
#include <gst/gst.h>
#include <gst/gl/gl.h>
#include <gst/check/gstcheck.h>
#include <gst/check/gstharness.h>
static void
replace_display (GstHarness * h)
{
GstContext *new_context;
GstGLDisplay *new_display;
GstGLContext *expected, *gl_context;
GstBuffer *buf;
/* replaces the GstGLDisplay used by @h with verification */
fail_unless_equals_int (GST_FLOW_OK, gst_harness_push_from_src (h));
buf = gst_harness_pull (h);
fail_unless (buf != NULL);
gst_clear_buffer (&buf);
g_object_get (G_OBJECT (h->element), "context", &gl_context, NULL);
fail_unless (gl_context != NULL);
gst_clear_object (&gl_context);
new_display = gst_gl_display_new ();
fail_unless (gst_gl_display_create_context (new_display, NULL, &expected,
NULL));
fail_unless (expected != NULL);
gst_gl_display_add_context (new_display, expected);
new_context = gst_context_new (GST_GL_DISPLAY_CONTEXT_TYPE, TRUE);
gst_context_set_gl_display (new_context, new_display);
gst_element_set_context (h->element, new_context);
gst_context_unref (new_context);
new_context = NULL;
fail_unless_equals_int (GST_FLOW_OK, gst_harness_push_from_src (h));
buf = gst_harness_pull (h);
fail_unless (buf != NULL);
gst_clear_buffer (&buf);
g_object_get (G_OBJECT (h->element), "context", &gl_context, NULL);
fail_unless (gl_context != NULL);
fail_unless (gl_context == expected);
fail_unless (new_display == gl_context->display);
gst_object_unref (expected);
gst_object_unref (gl_context);
gst_object_unref (new_display);
}
GST_START_TEST (test_glvideomixer_negotiate)
{
GstHarness *mix;
GstBuffer *buf;
mix = gst_harness_new_with_padnames ("glvideomixer", "sink_0", "src");
gst_harness_use_systemclock (mix);
gst_harness_set_blocking_push_mode (mix);
gst_harness_set_caps_str (mix,
"video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D",
"video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D");
gst_harness_add_src (mix, "gltestsrc", FALSE);
gst_harness_set_blocking_push_mode (mix->src_harness);
fail_unless_equals_int (GST_FLOW_OK, gst_harness_push_from_src (mix));
buf = gst_harness_pull (mix);
fail_unless (buf != NULL);
gst_clear_buffer (&buf);
gst_harness_teardown (mix);
}
GST_END_TEST;
GST_START_TEST (test_glvideomixer_display_replace)
{
GstHarness *mix;
mix = gst_harness_new_with_padnames ("glvideomixer", "sink_0", "src");
gst_harness_use_systemclock (mix);
gst_harness_set_blocking_push_mode (mix);
gst_harness_set_caps_str (mix,
"video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D",
"video/x-raw(memory:GLMemory),format=RGBA,width=1,height=1,framerate=25/1,texture-target=2D");
gst_harness_add_src (mix, "gltestsrc", FALSE);
gst_harness_set_blocking_push_mode (mix->src_harness);
replace_display (mix);
gst_harness_teardown (mix);
}
GST_END_TEST;
static Suite *
glmixer_suite (void)
{
Suite *s = suite_create ("glmixer");
TCase *tc = tcase_create ("general");
tcase_add_test (tc, test_glvideomixer_negotiate);
tcase_add_test (tc, test_glvideomixer_display_replace);
suite_add_tcase (s, tc);
return s;
}
int
main (int argc, char **argv)
{
Suite *s;
g_setenv ("GST_GL_XINITTHREADS", "1", TRUE);
gst_check_init (&argc, &argv);
s = glmixer_suite ();
return gst_check_run_suite (s, "glmixer", __FILE__);
}

View file

@ -102,6 +102,7 @@ if build_gstgl and host_machine.system() != 'windows'
[ 'pipelines/gl-launch-lines.c', not build_gstgl ],
[ 'elements/glfilter.c', not build_gstgl, [gstgl_dep]],
[ 'elements/glstereo.c', not build_gstgl, [gstgl_dep]],
[ 'elements/glmixer.c', not build_gstgl, [gstgl_dep]],
]
endif