agent: avoid leak of all turn refreshes when disposing the agent

With this patch we free all outstanding refreshes when the agent dispose
method is called, even those that are in the way to be discarded
asynchronously, when a stream is removed.

We also make the final user callback of the refresh proces synchronous,
so we don't have to deal with an heap use-after-free problem. This also
requires to order some parts of code.

diff --git a/agent/agent.c b/agent/agent.c
index e6132eb..a510be5 100644 (file)
--- a/agent/agent.c
+++ b/agent/agent.c
@@ -3615,13 +3615,13 @@ nice_agent_remove_stream (
   /* note: remove items with matching stream_ids from both lists */
   conn_check_prune_stream (agent, stream);
   discovery_prune_stream (agent, stream_id);
-  refresh_prune_stream_async (agent, stream,
-      (NiceTimeoutLockedCallback) on_stream_refreshes_pruned);
-
-  agent->pruning_streams = g_slist_prepend (agent->pruning_streams, stream);
 
   /* Remove the stream and signal its removal. */
   agent->streams = g_slist_remove (agent->streams, stream);
+  agent->pruning_streams = g_slist_prepend (agent->pruning_streams, stream);
+
+  refresh_prune_stream_async (agent, stream,
+      (NiceTimeoutLockedCallback) on_stream_refreshes_pruned);
 
   if (!agent->streams)
     priv_remove_keepalive_timer (agent);
@@ -5600,6 +5600,24 @@ nice_agent_dispose (GObject *object)
   g_slist_free (agent->local_addresses);
   agent->local_addresses = NULL;
 
+  if (agent->refresh_list)
+    g_warning ("Agent %p : We still have alive TURN refreshes. Consider "
+        "using nice_agent_close_async() to prune them before releasing the "
+        "agent.", agent);
+
+  /* We must free refreshes before closing streams because a refresh
+   * callback data may contain a pointer to a stream to be freed, when
+   * previously called in the context of a stream removal, by
+   * refresh_prune_stream_async()
+   */
+  for (i = agent->refresh_list; i;) {
+    GSList *next = i->next;
+    CandidateRefresh *refresh = i->data;
+
+    refresh_free (agent, refresh);
+    i = next;
+  }
+
   while (agent->streams) {
     NiceStream *s = agent->streams->data;
 
@@ -5624,20 +5642,6 @@ nice_agent_dispose (GObject *object)
     free_queued_signal (sig);
   }
 
-  if (agent->refresh_list)
-    g_warning ("Agent %p : We still have alive TURN refreshes. Consider "
-        "using nice_agent_close_async() to prune them before releasing the "
-        "agent.", agent);
-
-  for (i = agent->refresh_list; i;) {
-    GSList *next = i->next;
-    CandidateRefresh *refresh = i->data;
-
-    if (!refresh->disposing)
-      refresh_free (agent, refresh);
-    i = next;
-  }
-
   g_free (agent->stun_server_ip);
   agent->stun_server_ip = NULL;
 

diff --git a/agent/discovery.c b/agent/discovery.c
index aa3ab96..872c309 100644 (file)
--- a/agent/discovery.c
+++ b/agent/discovery.c
@@ -286,11 +286,7 @@ typedef struct {
 static void on_refresh_removed (RefreshPruneAsyncData *data)
 {
   if (data->items_to_free == 0 || --(data->items_to_free) == 0) {
-    GSource *timeout_source = NULL;
-    agent_timeout_add_with_context (data->agent, &timeout_source,
-        "Async refresh prune", 0, data->cb, data->user_data);
-
-    g_source_unref (timeout_source);
+    data->cb (data->agent, data->user_data);
     g_free (data);
   }
 }