Commit 5ed865fb authored by Philip Withnall's avatar Philip Withnall

agent: Clear existing GSource timeouts before adding new ones

Modify agent_timeout_add_with_context() to force destroying and freeing
of an existing GSource before overwriting it with a new one (probably
with an updated timeout period).

This fixes a case in priv_map_reply_to_relay_refresh() where the TURN
candidate refresh timer was being overwritten with a new one, without
the old one being destroyed. This lead to two timeouts existing, only
one of which would be destroyed when the CandidateRefresh struct was
freed, leaking the other one (in the main context) and allowing it to be
later dispatched with a dangling CandidateRefresh pointer.

The modification to agent_timeout_add_with_context() should prevent this
happening in new code in future.
parent e5fbdbe6
......@@ -206,8 +206,8 @@ void agent_signal_initial_binding_request_received (NiceAgent *agent, Stream *st
guint64 agent_candidate_pair_priority (NiceAgent *agent, NiceCandidate *local, NiceCandidate *remote);
GSource *agent_timeout_add_with_context (NiceAgent *agent, const gchar *name,
guint interval, GSourceFunc function, gpointer data);
void agent_timeout_add_with_context (NiceAgent *agent, GSource **out,
const gchar *name, guint interval, GSourceFunc function, gpointer data);
StunUsageIceCompatibility agent_to_ice_compatibility (NiceAgent *agent);
StunUsageTurnCompatibility agent_to_turn_compatibility (NiceAgent *agent);
......
......@@ -1813,7 +1813,7 @@ adjust_tcp_clock (NiceAgent *agent, Stream *stream, Component *component)
/* Prevent integer overflows */
if (interval < 0 || interval > G_MAXINT)
interval = G_MAXINT;
component->tcp_clock = agent_timeout_add_with_context (agent,
agent_timeout_add_with_context (agent, &component->tcp_clock,
"Pseudo-TCP clock", interval,
notify_pseudo_tcp_socket_clock, component);
}
......@@ -4964,20 +4964,36 @@ nice_agent_get_selected_socket (NiceAgent *agent, guint stream_id,
return g_socket;
}
GSource* agent_timeout_add_with_context (NiceAgent *agent, const gchar *name,
guint interval, GSourceFunc function, gpointer data)
/* Create a new timer GSource with the given @name, @interval, callback
* @function and @data, and assign it to @out, destroying and freeing any
* existing #GSource in @out first.
*
* This guarantees that a timer won’t be overwritten without being destroyed.
*/
void agent_timeout_add_with_context (NiceAgent *agent, GSource **out,
const gchar *name, guint interval, GSourceFunc function, gpointer data)
{
GSource *source;
g_return_val_if_fail (function != NULL, NULL);
g_return_if_fail (function != NULL);
g_return_if_fail (out != NULL);
/* Destroy any existing source. */
if (*out != NULL) {
g_source_destroy (*out);
g_source_unref (*out);
*out = NULL;
}
/* Create the new source. */
source = g_timeout_source_new (interval);
g_source_set_name (source, name);
g_source_set_callback (source, function, data, NULL);
g_source_attach (source, agent->main_context);
return source;
/* Return it! */
*out = source;
}
......
......@@ -541,16 +541,16 @@ static gboolean priv_conn_keepalive_retransmissions_tick (gpointer pointer)
nice_debug ("Agent %p : Retransmitting keepalive conncheck",
pair->keepalive.agent);
pair->keepalive.tick_source =
agent_timeout_add_with_context (pair->keepalive.agent,
"Pair keepalive", stun_timer_remainder (&pair->keepalive.timer),
priv_conn_keepalive_retransmissions_tick, pair);
agent_timeout_add_with_context (pair->keepalive.agent,
&pair->keepalive.tick_source,
"Pair keepalive", stun_timer_remainder (&pair->keepalive.timer),
priv_conn_keepalive_retransmissions_tick, pair);
break;
case STUN_USAGE_TIMER_RETURN_SUCCESS:
pair->keepalive.tick_source =
agent_timeout_add_with_context (pair->keepalive.agent,
"Pair keepalive", stun_timer_remainder (&pair->keepalive.timer),
priv_conn_keepalive_retransmissions_tick, pair);
agent_timeout_add_with_context (pair->keepalive.agent,
&pair->keepalive.tick_source,
"Pair keepalive", stun_timer_remainder (&pair->keepalive.timer),
priv_conn_keepalive_retransmissions_tick, pair);
break;
default:
/* Nothing to do. */
......@@ -667,21 +667,14 @@ static gboolean priv_conn_keepalive_tick_unlocked (NiceAgent *agent)
agent_socket_send (p->local->sockptr, &p->remote->addr,
buf_len, (gchar *)p->keepalive.stun_buffer);
if (p->keepalive.tick_source != NULL) {
g_source_destroy (p->keepalive.tick_source);
g_source_unref (p->keepalive.tick_source);
p->keepalive.tick_source = NULL;
}
p->keepalive.stream_id = stream->id;
p->keepalive.component_id = component->id;
p->keepalive.agent = agent;
p->keepalive.tick_source =
agent_timeout_add_with_context (p->keepalive.agent,
"Pair keepalive",
stun_timer_remainder (&p->keepalive.timer),
priv_conn_keepalive_retransmissions_tick, p);
agent_timeout_add_with_context (p->keepalive.agent,
&p->keepalive.tick_source, "Pair keepalive",
stun_timer_remainder (&p->keepalive.timer),
priv_conn_keepalive_retransmissions_tick, p);
} else {
++errors;
}
......@@ -824,12 +817,12 @@ static gboolean priv_turn_allocate_refresh_retransmissions_tick (gpointer pointe
agent_socket_send (cand->nicesock, &cand->server,
stun_message_length (&cand->stun_message), (gchar *)cand->stun_buffer);
cand->tick_source = agent_timeout_add_with_context (cand->agent,
agent_timeout_add_with_context (cand->agent, &cand->tick_source,
"Candidate TURN refresh", stun_timer_remainder (&cand->timer),
priv_turn_allocate_refresh_retransmissions_tick, cand);
break;
case STUN_USAGE_TIMER_RETURN_SUCCESS:
cand->tick_source = agent_timeout_add_with_context (cand->agent,
agent_timeout_add_with_context (cand->agent, &cand->tick_source,
"Candidate TURN refresh", stun_timer_remainder (&cand->timer),
priv_turn_allocate_refresh_retransmissions_tick, cand);
break;
......@@ -894,7 +887,7 @@ static void priv_turn_allocate_refresh_tick_unlocked (CandidateRefresh *cand)
agent_socket_send (cand->nicesock, &cand->server,
buffer_len, (gchar *)cand->stun_buffer);
cand->tick_source = agent_timeout_add_with_context (cand->agent,
agent_timeout_add_with_context (cand->agent, &cand->tick_source,
"Candidate TURN refresh", stun_timer_remainder (&cand->timer),
priv_turn_allocate_refresh_retransmissions_tick, cand);
}
......@@ -947,18 +940,16 @@ gboolean conn_check_schedule_next (NiceAgent *agent)
/* step: schedule timer if not running yet */
if (res && agent->conncheck_timer_source == NULL) {
agent->conncheck_timer_source =
agent_timeout_add_with_context (agent,
"Connectivity check schedule", agent->timer_ta,
priv_conn_check_tick, agent);
agent_timeout_add_with_context (agent, &agent->conncheck_timer_source,
"Connectivity check schedule", agent->timer_ta,
priv_conn_check_tick, agent);
}
/* step: also start the keepalive timer */
if (agent->keepalive_timer_source == NULL) {
agent->keepalive_timer_source =
agent_timeout_add_with_context (agent,
"Connectivity keepalive timeout", NICE_AGENT_TIMER_TR_DEFAULT,
priv_conn_keepalive_tick, agent);
agent_timeout_add_with_context (agent, &agent->keepalive_timer_source,
"Connectivity keepalive timeout", NICE_AGENT_TIMER_TR_DEFAULT,
priv_conn_keepalive_tick, agent);
}
nice_debug ("Agent %p : conn_check_schedule_next returning %d", agent, res);
......@@ -2519,9 +2510,9 @@ priv_add_new_turn_refresh (CandidateDiscovery *cdisco, NiceCandidate *relay_cand
/* step: also start the refresh timer */
/* refresh should be sent 1 minute before it expires */
cand->timer_source =
agent_timeout_add_with_context (agent, "Candidate TURN refresh",
(lifetime - 60) * 1000, priv_turn_allocate_refresh_tick, cand);
agent_timeout_add_with_context (agent, &cand->timer_source,
"Candidate TURN refresh",
(lifetime - 60) * 1000, priv_turn_allocate_refresh_tick, cand);
nice_debug ("timer source is : %p", cand->timer_source);
......@@ -2765,10 +2756,9 @@ static gboolean priv_map_reply_to_relay_refresh (NiceAgent *agent, StunMessage *
agent, cand, (int)res);
if (res == STUN_USAGE_TURN_RETURN_RELAY_SUCCESS) {
/* refresh should be sent 1 minute before it expires */
cand->timer_source =
agent_timeout_add_with_context (cand->agent,
"Candidate TURN refresh", (lifetime - 60) * 1000,
priv_turn_allocate_refresh_tick, cand);
agent_timeout_add_with_context (cand->agent, &cand->timer_source,
"Candidate TURN refresh", (lifetime - 60) * 1000,
priv_turn_allocate_refresh_tick, cand);
g_source_destroy (cand->tick_source);
g_source_unref (cand->tick_source);
......
......@@ -1169,10 +1169,9 @@ void discovery_schedule (NiceAgent *agent)
/* step: run first iteration immediately */
gboolean res = priv_discovery_tick_unlocked (agent);
if (res == TRUE) {
agent->discovery_timer_source =
agent_timeout_add_with_context (agent,
"Candidate discovery tick", agent->timer_ta,
priv_discovery_tick, agent);
agent_timeout_add_with_context (agent, &agent->discovery_timer_source,
"Candidate discovery tick", agent->timer_ta,
priv_discovery_tick, agent);
}
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment