Race between gst_rtsp_client_close() and client_watch_notify() leads to undefined behaviour

Submitted by Kseniya Vasilchuk

Link to original bug (#790909)

Description

Created attachment 364515
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.

Patch 364515, "prevent undefined behaviour":
prevent_undefined_behaviour.patch