display: add a specific lock for the list of windows

It's either this or replacing all the object lock usage in gldisplay
with a recursive mutex which is not backwards compatible

The failure case is effectively:
1. The user has locked the display object lock
2. a glcontext loses it's last ref and attempts to quit the window
3. gst_gl_window_quit() attempts to remove the window from the display
4. gst_gl_display_remove_window attempts to take the display object lock

The only concern with changing the locking for the window list in the
display is that gst_gl_display_create_window() has documentation requiring
the object lock to be held which must continue to work correctly.
This commit is contained in:
Matthew Waters 2019-11-03 00:46:49 +11:00 committed by GStreamer Merge Bot
parent b887db1efe
commit 3b552cc05a
2 changed files with 14 additions and 7 deletions

View file

@ -110,6 +110,8 @@ struct _GstGLDisplayPrivate
GMutex thread_lock; GMutex thread_lock;
GCond thread_cond; GCond thread_cond;
GMutex window_lock;
}; };
#define DEBUG_INIT \ #define DEBUG_INIT \
@ -194,6 +196,8 @@ gst_gl_display_init (GstGLDisplay * display)
g_mutex_init (&display->priv->thread_lock); g_mutex_init (&display->priv->thread_lock);
g_cond_init (&display->priv->thread_cond); g_cond_init (&display->priv->thread_cond);
g_mutex_init (&display->priv->window_lock);
display->priv->event_thread = g_thread_new ("gldisplay-event", display->priv->event_thread = g_thread_new ("gldisplay-event",
(GThreadFunc) _event_thread_main, display); (GThreadFunc) _event_thread_main, display);
@ -259,6 +263,7 @@ gst_gl_display_finalize (GObject * object)
g_cond_clear (&display->priv->thread_cond); g_cond_clear (&display->priv->thread_cond);
g_mutex_clear (&display->priv->thread_lock); g_mutex_clear (&display->priv->thread_lock);
g_mutex_clear (&display->priv->window_lock);
G_OBJECT_CLASS (gst_gl_display_parent_class)->finalize (object); G_OBJECT_CLASS (gst_gl_display_parent_class)->finalize (object);
} }
@ -576,10 +581,10 @@ gst_gl_display_create_context (GstGLDisplay * display,
* gst_gl_display_create_window: * gst_gl_display_create_window:
* @display: a #GstGLDisplay * @display: a #GstGLDisplay
* *
* It requires the display's object lock to be held.
*
* Returns: (transfer full): a new #GstGLWindow for @display or %NULL. * Returns: (transfer full): a new #GstGLWindow for @display or %NULL.
*/ */
/* XXX: previous versions had documentation requiring the OBJECT lock to be
* held when this fuction is called so that needs to always work. */
GstGLWindow * GstGLWindow *
gst_gl_display_create_window (GstGLDisplay * display) gst_gl_display_create_window (GstGLDisplay * display)
{ {
@ -590,11 +595,13 @@ gst_gl_display_create_window (GstGLDisplay * display)
klass = GST_GL_DISPLAY_GET_CLASS (display); klass = GST_GL_DISPLAY_GET_CLASS (display);
g_return_val_if_fail (klass->create_window != NULL, NULL); g_return_val_if_fail (klass->create_window != NULL, NULL);
g_mutex_lock (&display->priv->window_lock);
window = klass->create_window (display); window = klass->create_window (display);
if (window) { if (window) {
display->windows = g_list_prepend (display->windows, window); display->windows = g_list_prepend (display->windows, window);
} }
g_mutex_unlock (&display->priv->window_lock);
GST_DEBUG_OBJECT (display, "Adding window %" GST_PTR_FORMAT GST_DEBUG_OBJECT (display, "Adding window %" GST_PTR_FORMAT
" (%p) to internal list", window, window); " (%p) to internal list", window, window);
@ -622,7 +629,7 @@ gst_gl_display_remove_window (GstGLDisplay * display, GstGLWindow * window)
gboolean ret = FALSE; gboolean ret = FALSE;
GList *l; GList *l;
GST_OBJECT_LOCK (display); g_mutex_lock (&display->priv->window_lock);
l = g_list_find (display->windows, window); l = g_list_find (display->windows, window);
if (l) { if (l) {
display->windows = g_list_delete_link (display->windows, l); display->windows = g_list_delete_link (display->windows, l);
@ -630,7 +637,7 @@ gst_gl_display_remove_window (GstGLDisplay * display, GstGLWindow * window)
} }
GST_DEBUG_OBJECT (display, "Removing window %" GST_PTR_FORMAT GST_DEBUG_OBJECT (display, "Removing window %" GST_PTR_FORMAT
" (%p) from internal list", window, window); " (%p) from internal list", window, window);
GST_OBJECT_UNLOCK (display); g_mutex_unlock (&display->priv->window_lock);
return ret; return ret;
} }
@ -687,14 +694,14 @@ gst_gl_display_retrieve_window (GstGLDisplay * display, gpointer data,
GstGLWindow *ret = NULL; GstGLWindow *ret = NULL;
GList *l; GList *l;
GST_OBJECT_LOCK (display); g_mutex_lock (&display->priv->window_lock);
l = g_list_find_custom (display->windows, data, compare_func); l = g_list_find_custom (display->windows, data, compare_func);
if (l) if (l)
ret = gst_object_ref (l->data); ret = gst_object_ref (l->data);
GST_DEBUG_OBJECT (display, "Found window %" GST_PTR_FORMAT GST_DEBUG_OBJECT (display, "Found window %" GST_PTR_FORMAT
" (%p) in internal list", ret, ret); " (%p) in internal list", ret, ret);
GST_OBJECT_UNLOCK (display); g_mutex_unlock (&display->priv->window_lock);
return ret; return ret;
} }

View file

@ -83,7 +83,7 @@ struct _GstGLDisplay
GstGLDisplayType type; GstGLDisplayType type;
/*< protected >*/ /*< protected >*/
GList *windows; /* OBJECT lock */ GList *windows; /* internal lock, use *_window functions instead */
GMainContext *main_context; GMainContext *main_context;
GMainLoop *main_loop; GMainLoop *main_loop;
GSource *event_source; GSource *event_source;