conncheck: define a property for a final idle timeout

This final idle timeout is renamed from the
NICE_AGENT_MAX_TIMER_GRACE_PERIOD macro, and keeps its semantic.

We also increase the default value of this timeout from 1 second to 5
seconds.  This is useful for the sipe pidgin plugin that has to deal
with SfB agents, that may take some time in controlling mode before
choosing and testing the nominated pair

3 files changed, 65 insertions, 19 deletions

diff --git a/agent/agent-priv.h b/agent/agent-priv.h
index ee1000e..4b47252 100644
--- a/agent/agent-priv.h
+++ b/agent/agent-priv.h
@@ -122,18 +122,6 @@ nice_input_message_iter_compare (const NiceInputMessageIter *a,
   ((obj)->compatibility == NICE_COMPATIBILITY_RFC5245 || \
   (obj)->compatibility == NICE_COMPATIBILITY_OC2007R2)
 
-/* A grace period before declaring a component as failed, in msecs. This
- * delay is added to reduce the chance to see the agent receiving new
- * stun activity just after the conncheck list has been declared failed,
- * reactiviting conncheck activity, and causing a (valid) state
- * transitions like that: connecting -> failed -> connecting ->
- * connected -> ready.
- * Such transitions are not buggy per-se, but may break the
- * test-suite, that counts precisely the number of time each state
- * has been set, and doesnt expect these transcient failed states.
- */
-#define NICE_AGENT_MAX_TIMER_GRACE_PERIOD 1000
-
 struct _NiceAgent
 {
   GObject parent;                 /* gobject pointer */
@@ -157,6 +145,7 @@ struct _NiceAgent
   guint stun_reliable_timeout;    /* property: stun reliable timeout */
   NiceNominationMode nomination_mode; /* property: Nomination mode */
   gboolean support_renomination;  /* property: support RENOMINATION STUN attribute */
+  guint idle_timeout;             /* property: conncheck timeout before stop */
 
   GSList *local_addresses;        /* list of NiceAddresses for local
 				     interfaces */
@@ -193,7 +182,7 @@ struct _NiceAgent
   gboolean use_ice_tcp;
   gboolean use_ice_trickle;
 
-  guint conncheck_timer_grace_period; /* ongoing delay before timer stop */
+  guint conncheck_ongoing_idle_delay; /* ongoing delay before timer stop */
   gboolean controlling_mode;          /* controlling mode used by the
                                          conncheck */
   /* XXX: add pointer to internal data struct for ABI-safe extensions */
diff --git a/agent/agent.c b/agent/agent.c
index cc1b5c8..b232690 100644
--- a/agent/agent.c
+++ b/agent/agent.c
@@ -81,9 +81,11 @@
 
 #define DEFAULT_STUN_PORT  3478
 #define DEFAULT_UPNP_TIMEOUT 200  /* milliseconds */
+#define DEFAULT_IDLE_TIMEOUT 5000 /* milliseconds */
 
 #define MAX_TCP_MTU 1400 /* Use 1400 because of VPNs and we assume IEE 802.3 */
 
+
 static void
 nice_debug_input_message_composition (const NiceInputMessage *messages,
     guint n_messages);
@@ -120,6 +122,7 @@ enum
   PROP_NOMINATION_MODE,
   PROP_ICE_TRICKLE,
   PROP_SUPPORT_RENOMINATION,
+  PROP_IDLE_TIMEOUT,
 };
 
 
@@ -485,6 +488,51 @@ nice_agent_class_init (NiceAgentClass *klass)
          G_PARAM_READWRITE));
 
   /**
+   * NiceAgent:idle-timeout
+   *
+   * A final timeout in msec, launched when the agent becomes idle,
+   * before stopping its activity.
+   *
+   * This timer will delay the decision to set a component as failed.
+   * This delay is added to reduce the chance to see the agent receiving
+   * new stun activity just after the conncheck list has been declared
+   * failed (some valid pairs, no nominated pair, and no in-progress
+   * pairs), reactiviting conncheck activity, and causing a (valid)
+   * state transitions like that: connecting -> failed -> connecting ->
+   * connected -> ready.  Such transitions are not buggy per-se, but may
+   * break the test-suite, that counts precisely the number of time each
+   * state has been set, and doesnt expect these transcient failed
+   * states.
+   *
+   * This timer is also useful when the agent is in controlled mode and
+   * the other controlling peer takes some time to elect its nominated
+   * pair (this may be the case for SfB peers).
+   *
+   * This timer is *NOT* part if the RFC5245, as this situation is not
+   * covered in sect 8.1.2 "Updating States", but deals with a real
+   * use-case, where a controlled agent can not wait forever for the
+   * other peer to make a nomination decision.
+   *
+   * Also note that the value of this timeout will not delay the
+   * emission of 'connected' and 'ready' agent signals, and will not
+   * slow down the behaviour of the agent when the peer agent works
+   * in a timely manner.
+   *
+   * Since: 0.1.17
+   */
+
+  g_object_class_install_property (gobject_class, PROP_IDLE_TIMEOUT,
+      g_param_spec_uint (
+         "idle-timeout",
+         "Timeout before stopping the agent when being idle",
+         "A final timeout in msecs, launched when the agent becomes idle, "
+         "with no in-progress pairs to wait for, before stopping its activity, "
+         "and declaring a component as failed in needed.",
+         50, 60000,
+	 DEFAULT_IDLE_TIMEOUT,
+         G_PARAM_READWRITE | G_PARAM_CONSTRUCT));
+
+  /**
    * NiceAgent:proxy-ip:
    *
    * The proxy server IP used to bypass a proxy firewall
@@ -1203,6 +1251,7 @@ nice_agent_init (NiceAgent *agent)
   agent->max_conn_checks = NICE_AGENT_MAX_CONNECTIVITY_CHECKS_DEFAULT;
   agent->nomination_mode = NICE_NOMINATION_MODE_AGGRESSIVE;
   agent->support_renomination = FALSE;
+  agent->idle_timeout = DEFAULT_IDLE_TIMEOUT;
 
   agent->discovery_list = NULL;
   agent->discovery_unsched_items = 0;
@@ -1327,6 +1376,10 @@ nice_agent_get_property (
       g_value_set_boolean (value, agent->support_renomination);
       break;
 
+    case PROP_IDLE_TIMEOUT:
+      g_value_set_uint (value, agent->idle_timeout);
+      break;
+
     case PROP_PROXY_IP:
       g_value_set_string (value, agent->proxy_ip);
       break;
@@ -1543,6 +1596,10 @@ nice_agent_set_property (
       agent->support_renomination = g_value_get_boolean (value);
       break;
 
+    case PROP_IDLE_TIMEOUT:
+      agent->idle_timeout = g_value_get_uint (value);
+      break;
+
     case PROP_PROXY_IP:
       g_free (agent->proxy_ip);
       agent->proxy_ip = g_value_dup_string (value);
diff --git a/agent/conncheck.c b/agent/conncheck.c
index 369dd0a..df298ea 100644
--- a/agent/conncheck.c
+++ b/agent/conncheck.c
@@ -1105,7 +1105,7 @@ conn_check_stop (NiceAgent *agent)
   g_source_destroy (agent->conncheck_timer_source);
   g_source_unref (agent->conncheck_timer_source);
   agent->conncheck_timer_source = NULL;
-  agent->conncheck_timer_grace_period = 0;
+  agent->conncheck_ongoing_idle_delay = 0;
 }
 
 
@@ -1192,18 +1192,18 @@ static gboolean priv_conn_check_tick_agent_locked (NiceAgent *agent,
    * failed. Components marked connected, and then ready follow another
    * code path, and are not concerned by this grace period.
    */
-  if (!keep_timer_going && agent->conncheck_timer_grace_period == 0)
+  if (!keep_timer_going && agent->conncheck_ongoing_idle_delay == 0)
     nice_debug ("Agent %p : waiting %d msecs before checking "
-        "for failed components.", agent, NICE_AGENT_MAX_TIMER_GRACE_PERIOD);
+        "for failed components.", agent, agent->idle_timeout);
 
   if (keep_timer_going)
-    agent->conncheck_timer_grace_period = 0;
+    agent->conncheck_ongoing_idle_delay = 0;
   else
-    agent->conncheck_timer_grace_period += agent->timer_ta;
+    agent->conncheck_ongoing_idle_delay += agent->timer_ta;
 
   /* step: stop timer if no work left */
   if (!keep_timer_going &&
-      agent->conncheck_timer_grace_period >= NICE_AGENT_MAX_TIMER_GRACE_PERIOD) {
+      agent->conncheck_ongoing_idle_delay >= agent->idle_timeout) {
     nice_debug ("Agent %p : checking for failed components now.", agent);
     for (i = agent->streams; i; i = i->next) {
       NiceStream *stream = i->data;