diff options
author | He Junyan <junyan.he@intel.com> | 2020-11-03 20:19:16 +0800 |
---|---|---|
committer | Tim-Philipp Müller <tim@centricular.com> | 2020-11-05 11:52:34 +0000 |
commit | 2cfa929882d36aee9fba5f3be1dad6863f444e5d (patch) | |
tree | bdb941c126b1f0fd9d1c707bef66caa26937464f | |
parent | 3b9b478f8ba0bedbb587a565b1327f79b1b9a616 (diff) | |
download | gstreamer-plugins-base-2cfa929882d36aee9fba5f3be1dad6863f444e5d.tar.gz |
gluploadelement: Avoid race condition of inside upload creation.
The operations for the inside GstGLUploadElement->upload have race
condition. The _transform_caps() will creates this object if it does
not exist, while the _stop() and change_state() can destroy this object.
The _transform_caps() is called by the gst_base_transform_query(),
so it does not hold the stream lock. It may use the upload while the
_stop() and change_state() has already destroy that object, and then
crash.
Fix: #645
Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/920>
-rw-r--r-- | ext/gl/gstgluploadelement.c | 59 |
1 files changed, 44 insertions, 15 deletions
diff --git a/ext/gl/gstgluploadelement.c b/ext/gl/gstgluploadelement.c index 332ffef97..fab026bb6 100644 --- a/ext/gl/gstgluploadelement.c +++ b/ext/gl/gstgluploadelement.c @@ -66,13 +66,25 @@ GST_STATIC_PAD_TEMPLATE ("src", GST_STATIC_CAPS ("video/x-raw(ANY)")); static void +_gst_gl_upload_element_clear_upload (GstGLUploadElement * upload) +{ + GstGLUpload *ul = NULL; + + GST_OBJECT_LOCK (upload); + ul = upload->upload; + upload->upload = NULL; + GST_OBJECT_UNLOCK (upload); + + if (ul) + gst_object_unref (ul); +} + +static void gst_gl_upload_element_finalize (GObject * object) { GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (object); - if (upload->upload) - gst_object_unref (upload->upload); - upload->upload = NULL; + _gst_gl_upload_element_clear_upload (upload); G_OBJECT_CLASS (parent_class)->finalize (object); } @@ -123,10 +135,7 @@ gst_gl_upload_element_stop (GstBaseTransform * bt) { GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (bt); - if (upload->upload) { - gst_object_unref (upload->upload); - upload->upload = NULL; - } + _gst_gl_upload_element_clear_upload (upload); return GST_BASE_TRANSFORM_CLASS (parent_class)->stop (bt); } @@ -152,16 +161,39 @@ _gst_gl_upload_element_transform_caps (GstBaseTransform * bt, GstGLBaseFilter *base_filter = GST_GL_BASE_FILTER (bt); GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (bt); GstGLContext *context; + GstGLUpload *ul = NULL; + GstCaps *ret_caps; if (base_filter->display && !gst_gl_base_filter_find_gl_context (base_filter)) return NULL; context = GST_GL_BASE_FILTER (bt)->context; - if (upload->upload == NULL) - upload->upload = gst_gl_upload_new (context); - return gst_gl_upload_transform_caps (upload->upload, context, direction, caps, - filter); + GST_OBJECT_LOCK (upload); + if (upload->upload == NULL) { + GST_OBJECT_UNLOCK (upload); + + ul = gst_gl_upload_new (context); + + GST_OBJECT_LOCK (upload); + if (upload->upload) { + gst_object_unref (ul); + ul = upload->upload; + } else { + upload->upload = ul; + } + } else { + ul = upload->upload; + } + + gst_object_ref (ul); + GST_OBJECT_UNLOCK (upload); + + ret_caps = + gst_gl_upload_transform_caps (ul, context, direction, caps, filter); + gst_object_unref (ul); + + return ret_caps; } static gboolean @@ -296,10 +328,7 @@ gst_gl_upload_element_change_state (GstElement * element, switch (transition) { case GST_STATE_CHANGE_READY_TO_NULL: - if (upload->upload) { - gst_object_unref (upload->upload); - upload->upload = NULL; - } + _gst_gl_upload_element_clear_upload (upload); break; default: break; |