From 9e37fa55bf223a42f50f0c495fcbe01a5532dee3 Mon Sep 17 00:00:00 2001
From: He Junyan <junyan.he@intel.com>
Date: Tue, 3 Nov 2020 20:19:16 +0800
Subject: [PATCH] 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>
---
 ext/gl/gstgluploadelement.c | 59 +++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/ext/gl/gstgluploadelement.c b/ext/gl/gstgluploadelement.c
index 332ffef97f..fab026bb62 100644
--- a/ext/gl/gstgluploadelement.c
+++ b/ext/gl/gstgluploadelement.c
@@ -65,14 +65,26 @@ GST_STATIC_PAD_TEMPLATE ("src",
     GST_PAD_ALWAYS,
     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;