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/913>
This commit is contained in:
He Junyan 2020-11-03 20:19:16 +08:00
parent 576f950e18
commit 9e37fa55bf

View file

@ -65,14 +65,26 @@ GST_STATIC_PAD_TEMPLATE ("src",
GST_PAD_ALWAYS, GST_PAD_ALWAYS,
GST_STATIC_CAPS ("video/x-raw(ANY)")); 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 static void
gst_gl_upload_element_finalize (GObject * object) gst_gl_upload_element_finalize (GObject * object)
{ {
GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (object); GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (object);
if (upload->upload) _gst_gl_upload_element_clear_upload (upload);
gst_object_unref (upload->upload);
upload->upload = NULL;
G_OBJECT_CLASS (parent_class)->finalize (object); 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); GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (bt);
if (upload->upload) { _gst_gl_upload_element_clear_upload (upload);
gst_object_unref (upload->upload);
upload->upload = NULL;
}
return GST_BASE_TRANSFORM_CLASS (parent_class)->stop (bt); 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); GstGLBaseFilter *base_filter = GST_GL_BASE_FILTER (bt);
GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (bt); GstGLUploadElement *upload = GST_GL_UPLOAD_ELEMENT (bt);
GstGLContext *context; GstGLContext *context;
GstGLUpload *ul = NULL;
GstCaps *ret_caps;
if (base_filter->display && !gst_gl_base_filter_find_gl_context (base_filter)) if (base_filter->display && !gst_gl_base_filter_find_gl_context (base_filter))
return NULL; return NULL;
context = GST_GL_BASE_FILTER (bt)->context; 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, GST_OBJECT_LOCK (upload);
filter); 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 static gboolean
@ -296,10 +328,7 @@ gst_gl_upload_element_change_state (GstElement * element,
switch (transition) { switch (transition) {
case GST_STATE_CHANGE_READY_TO_NULL: case GST_STATE_CHANGE_READY_TO_NULL:
if (upload->upload) { _gst_gl_upload_element_clear_upload (upload);
gst_object_unref (upload->upload);
upload->upload = NULL;
}
break; break;
default: default:
break; break;