ref counting issue fix for Context in gst-libs/gst/gl/gstglcontext.c

[Matthew Waters]

So essentially this takes and keeps an extra ref while the context is
active on that thread.  That makes sense.

Can you provide a git format-patch against master and attach it to a new
bug in bugzilla
https://bugzilla.gnome.org/enter_bug.cgi?product=GStreamer&component=gst-plugins-bad,
we can attribute your work and review the change properly :)

Cheers
-Matt

On 23/02/17 17:42, Frank VanZile wrote:
> There is a ref counting issue in gst-libs/gst/gl/gstglcontext.c.
> _GstGLContextPrivate holds onto GThread's without proper ref
> counting.  when a thread was released I would get exceptions.
> see my previous email "heap corruption in gstreamer when IP camera
> with an rtp stream disconnects (losses network)"
>
>
>
> --- a/gst-libs/gst/gl/gstglcontext.c
> +++ b/gst-libs/gst/gl/gstglcontext.c
> @@ -371,7 +371,7 @@ gst_gl_context_new (GstGLDisplay * display)
>   * @context_type: a #GstGLPlatform specifying the type of context in
> @handle
>   * @available_apis: a #GstGLAPI containing the available OpenGL apis
> in @handle
>   *
> - * Wraps an existing OpenGL context into a #GstGLContext.
> + * Wraps an existing OpenGL context into a #GstGLContext.
>   *
>   * Note: The caller is responsible for ensuring that the OpenGL context
>   * represented by @handle stays alive while the returned
> #GstGLContext is
> @@ -668,19 +668,37 @@ gst_gl_context_finalize (GObject * object)
>          g_cond_wait (&context->priv->destroy_cond,
> &context->priv->render_lock);
>        GST_INFO_OBJECT (context, "gl thread joined");
>
> -      g_thread_unref (context->priv->gl_thread);
> -      context->priv->gl_thread = NULL;
> +      if (context->priv->gl_thread) {
> +        g_thread_unref (context->priv->gl_thread);
> +        context->priv->gl_thread = NULL;
> +      }
>      }
>      g_mutex_unlock (&context->priv->render_lock);
>
>      gst_gl_window_set_close_callback (context->window, NULL, NULL,
> NULL);
>      gst_object_unref (context->window);
> +    context->window = NULL;
>    }
>
> -  if (context->priv->sharegroup)
> +  if (context->priv->active_thread) {
> +      g_thread_unref (context->priv->active_thread);
> +      context->priv->active_thread = NULL;
> +  }
> +
> +  if (context->priv->gl_thread) {
> +    g_thread_unref (context->priv->gl_thread);
> +    context->priv->gl_thread = NULL;
> +  }
> +
> +  if (context->priv->sharegroup) {
>      _context_share_group_unref (context->priv->sharegroup);
> +    context->priv->sharegroup = NULL;
> +  }
>
> -  gst_object_unref (context->display);
> +  if (context->display) {
> +    gst_object_unref (context->display);
> +    context->display = NULL;
> +  }
>
>    if (context->gl_vtable) {
>      g_slice_free (GstGLFuncs, context->gl_vtable);
> @@ -729,10 +747,19 @@ gst_gl_context_activate (GstGLContext * context,
> gboolean activate)
>    result = context_class->activate (context, activate);
>
>    if (result && activate) {
> -    context->priv->active_thread = g_thread_self ();
> +    GThread *saveOldThread = context->priv->active_thread; //save the
> old thread
> +    context->priv->active_thread = g_thread_self ();  // ref count is
> 1, does not increment count
> +    g_thread_ref (context->priv->active_thread); //add ref count
> +    if (saveOldThread) {
> +        g_thread_unref (saveOldThread); // release old thread
> +    }
> +
>      g_private_set (&current_context_key, context);
>    } else {
> -    context->priv->active_thread = NULL;
> +    if (context->priv->active_thread) {
> +      g_thread_unref (context->priv->active_thread);
> +      context->priv->active_thread = NULL;
> +    }
>      g_private_set (&current_context_key, NULL);
>    }
>    GST_OBJECT_UNLOCK (context);
> @@ -1002,7 +1029,7 @@ gst_gl_context_create (GstGLContext * context,
>            _context_share_group_ref (other_context->priv->sharegroup);
>
>      context->priv->gl_thread = g_thread_new ("gstglcontext",
> -        (GThreadFunc) gst_gl_context_create_thread, context);
> +        (GThreadFunc) gst_gl_context_create_thread, context); // ref
> count 2
>
>      while (!context->priv->created)
>        g_cond_wait (&context->priv->create_cond,
> &context->priv->render_lock);
> @@ -1728,10 +1755,19 @@ gst_gl_wrapped_context_get_gl_platform
> (GstGLContext * context)
>  static gboolean
>  gst_gl_wrapped_context_activate (GstGLContext * context, gboolean
> activate)
>  {
> -  if (activate)
> -    context->priv->gl_thread = g_thread_self ();
> -  else
> -    context->priv->gl_thread = NULL;
> +  if (activate) {
> +    GThread *saveOldThread = context->priv->gl_thread; // save old
> thread
> +    context->priv->gl_thread = g_thread_self (); // ref count is one
> +    g_thread_ref (context->priv->gl_thread); // add ref count
> +    if (saveOldThread) {
> +      g_thread_unref (saveOldThread); // release old thread
> +    }
> +  } else {
> +    if (context->priv->gl_thread) {
> +      g_thread_unref (context->priv->gl_thread);
> +      context->priv->gl_thread = NULL;
> +    }
> +  }
>
>    return TRUE;
>  }
>


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 516 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/gstreamer-devel/attachments/20170223/00f66e6c/attachment.sig>