summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFabrice Bellet <fabrice@bellet.info>2020-12-19 17:56:16 +0100
committerOlivier CrĂȘte <olivier.crete@ocrete.ca>2021-04-20 19:37:08 +0000
commitcaf9f1d8b1d4675c8c88f8f6fa04d3ff5e27f09a (patch)
treecdf24c5e44d56f0ba65e9d083faa991b358c9919
parente7b0b78650d5678d38fcede0b4feffca57c067c5 (diff)
downloadlibnice-caf9f1d8b1d4675c8c88f8f6fa04d3ff5e27f09a.tar.gz
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.
-rw-r--r--agent/agent.c40
-rw-r--r--agent/discovery.c6
2 files changed, 23 insertions, 23 deletions
diff --git a/agent/agent.c b/agent/agent.c
index e6132eb..a510be5 100644
--- 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
--- 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);
}
}