From 7f70d7a9450b321585fbfd1eb977548d4264b2a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Mon, 7 Jan 2019 14:08:00 +0200 Subject: [PATCH 1/2] bin: Hold the state lock while removing elements from a bin We need to take the state lock here to ensure that we're not currently just before setting the state of this child element. Otherwise it can happen that we removed the element here and e.g. set it to NULL state, and shortly afterwards have another thread set it to a higher state again as part of a state change for the whole bin. When adding an element to the bin this is not needed as we require callers to always ensure after adding to the bin that the new element is set to the correct state. --- gst/gstbin.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/gst/gstbin.c b/gst/gstbin.c index e5d4ecd4c4b..d9060c84c0e 100644 --- a/gst/gstbin.c +++ b/gst/gstbin.c @@ -1864,6 +1864,19 @@ gst_bin_remove (GstBin * bin, GstElement * element) if (G_UNLIKELY (bclass->remove_element == NULL)) goto no_function; + /* We need to take the state lock here to ensure that we're + * not currently just before setting the state of this child + * element. Otherwise it can happen that we removed the element + * here and e.g. set it to NULL state, and shortly afterwards + * have another thread set it to a higher state again as part of + * a state change for the whole bin. + * + * When adding an element to the bin this is not needed as we + * require callers to always ensure after adding to the bin that + * the new element is set to the correct state. + */ + GST_STATE_LOCK (bin); + GST_CAT_DEBUG (GST_CAT_PARENTAGE, "removing element %s from bin %s", GST_ELEMENT_NAME (element), GST_ELEMENT_NAME (bin)); @@ -1871,6 +1884,8 @@ gst_bin_remove (GstBin * bin, GstElement * element) result = bclass->remove_element (bin, element); GST_TRACER_BIN_REMOVE_POST (bin, result); + GST_STATE_UNLOCK (bin); + return result; /* ERROR handling */ -- GitLab From cacc834d8f97533c268232e60e18985dda37abea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Mon, 7 Jan 2019 14:08:25 +0200 Subject: [PATCH 2/2] element: Add note about racyness to gst_element_set_locked_state() This is racy if the state lock of the parent bin is not taken. The parent bin might've just checked the flag in another thread and as the next step proceed to change the child element's state. --- gst/gstelement.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gst/gstelement.c b/gst/gstelement.c index 4078fa7219a..e186a09353c 100644 --- a/gst/gstelement.c +++ b/gst/gstelement.c @@ -2241,6 +2241,10 @@ gst_element_is_locked_state (GstElement * element) * Locks the state of an element, so state changes of the parent don't affect * this element anymore. * + * Note that this is racy if the state lock of the parent bin is not taken. + * The parent bin might've just checked the flag in another thread and as the + * next step proceed to change the child element's state. + * * MT safe. * * Returns: %TRUE if the state was changed, %FALSE if bad parameters were given -- GitLab