[Bug 790909] New: Race between gst_rtsp_client_close() and client_watch_notify() leads to undefined behaviour

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=790909

            Bug ID: 790909
           Summary: Race between gst_rtsp_client_close() and
                    client_watch_notify() leads to undefined behaviour
    Classification: Platform
           Product: GStreamer
           Version: 1.11.x
                OS: Windows
            Status: NEW
          Severity: normal
          Priority: Normal
         Component: gst-rtsp-server
          Assignee: gstreamer-bugs at lists.freedesktop.org
          Reporter: vasilchukkseniia at gmail.com
        QA Contact: gstreamer-bugs at lists.freedesktop.org
     GNOME version: ---

Created attachment 364515
  --> https://bugzilla.gnome.org/attachment.cgi?id=364515&action=edit
prevent undefined behaviour

The failed preroll causes client_watch_notify() which calls
rtsp_ctrl_timeout_remove() function.
This function calls g_main_context_find_source_by_id() which use
priv->watch_context to find the source to destroy it.

But if in the same time gst_rtsp_client_close() was called (e.g. by
gst_rtsp_server_client_filter() function) it can unref and set
priv->watch_context to NULL before rtsp_ctrl_timeout_remove().
In this case manual on g_main_context_find_source_by_id() says: 

if GMainContext is NULL, the default context will be used. But we don't use
default context in rtsp client so we trying to find non-existent source, it
means:

//-> from https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html
It is a programmer error to attempt to lookup a non-existent source.
More specifically: source IDs can be reissued after a source has been destroyed
and therefore it is never valid to use this function with a source ID which may
have already been removed. An example is when scheduling an idle to run in
another thread with g_idle_add(): the idle may already have run and been
removed by the time this function is called on its (now invalid) source ID.
This source ID may have been reissued, leading to the operation being performed
against the wrong source.
//<- 

So we have undefined behaviour here.

P.S.
If we are lucky and default context does not have source with such id, it
causes "g_source_destroy: assertion 'source != NULL' failed".

I've attached a patch to prevent use g_main_context_find_source_by_id() with
NULL GMainContext. Please watch it.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.