Avoid memory bloat on ICE restart (#2903)

pjlib/include/pj/lock.h

@@ -194,6 +194,15 @@ typedef struct pj_grp_lock_config
 } pj_grp_lock_config;
 
 
+/**
+ * The group lock destroy handler, a destructor function called when
+ * a group lock is about to be destroyed.
+ *
+ * @param member	A pointer to be passed to the handler.
+ */
+typedef void (*pj_grp_lock_handler)(void *member);
+
+
 /**
  * Initialize the config with the default values.
  *
@@ -235,7 +244,7 @@ PJ_DECL(pj_status_t) pj_grp_lock_create(pj_pool_t *pool,
 PJ_DECL(pj_status_t) pj_grp_lock_create_w_handler(pj_pool_t *pool,
                                         	  const pj_grp_lock_config *cfg,
                                         	  void *member,
-                                                  void (*handler)(void *member),
+                                                  pj_grp_lock_handler handler,
                                         	  pj_grp_lock_t **p_grp_lock);
 
 /**
@@ -303,7 +312,7 @@ PJ_DECL(pj_status_t) pj_grp_lock_release( pj_grp_lock_t *grp_lock);
 PJ_DECL(pj_status_t) pj_grp_lock_add_handler(pj_grp_lock_t *grp_lock,
                                              pj_pool_t *pool,
                                              void *member,
-                                             void (*handler)(void *member));
+                                             pj_grp_lock_handler handler);
 
 /**
  * Remove previously registered handler. All parameters must be the same
@@ -317,7 +326,7 @@ PJ_DECL(pj_status_t) pj_grp_lock_add_handler(pj_grp_lock_t *grp_lock,
  */
 PJ_DECL(pj_status_t) pj_grp_lock_del_handler(pj_grp_lock_t *grp_lock,
                                              void *member,
-                                             void (*handler)(void *member));
+                                             pj_grp_lock_handler handler);
 
 /**
  * Increment reference counter to prevent the group lock grom being destroyed.

pjnath/include/pjnath/ice_session.h

@@ -861,6 +861,27 @@ PJ_DECL(pj_status_t) pj_ice_sess_get_options(pj_ice_sess *ice,
 PJ_DECL(pj_status_t) pj_ice_sess_set_options(pj_ice_sess *ice,
 					     const pj_ice_sess_options *opt);
 
+
+/**
+ * Detach ICE session from group lock. This will delete ICE session group lock
+ * handler.
+ *
+ * This function is useful when application creates an ICE session with
+ * group lock and later it needs to recreate ICE session (e.g: for ICE
+ * restart) so the previous ICE session resources can be released manually
+ * (by calling the group lock handler) without waiting for the group lock
+ * destroy to avoid memory bloat.
+ *
+ * @param ice		ICE session instance.
+ * @param handler	Pointer to receive the group lock handler of
+ *			this ICE session.
+ *
+ * @return		PJ_SUCCESS on success.
+ */
+PJ_DECL(pj_status_t) pj_ice_sess_detach_grp_lock(pj_ice_sess *ice,
+						 pj_grp_lock_handler *handler);
+
+
 /**
  * Destroy ICE session. This will cancel any connectivity checks currently
  * running, if any, and any other events scheduled by this session, as well

pjnath/src/pjnath/ice_session.c

@@ -535,6 +535,22 @@ PJ_DEF(pj_status_t) pj_ice_sess_destroy(pj_ice_sess *ice)
 }
 
 
+/*
+ * Detach ICE session from group lock.
+ */
+PJ_DEF(pj_status_t) pj_ice_sess_detach_grp_lock(pj_ice_sess *ice,
+						pj_grp_lock_handler *handler)
+{
+    PJ_ASSERT_RETURN(ice && handler, PJ_EINVAL);
+
+    pj_grp_lock_acquire(ice->grp_lock);
+    pj_grp_lock_del_handler(ice->grp_lock, ice, &ice_on_destroy);
+    *handler = &ice_on_destroy;
+    pj_grp_lock_release(ice->grp_lock);
+    return PJ_SUCCESS;
+}
+
+
 /*
  * Change session role. 
  */

pjnath/src/pjnath/ice_strans.c

@@ -208,6 +208,8 @@ struct pj_ice_strans
 
     pj_ice_strans_state	     state;	/**< Session state.		*/
     pj_ice_sess		    *ice;	/**< ICE session.		*/
+    pj_ice_sess		    *ice_prev;	/**< Previous ICE session.	*/
+    pj_grp_lock_handler	     ice_prev_hndlr; /**< Handler of prev ICE	*/
     pj_time_val		     start_time;/**< Time when ICE was started	*/
 
     unsigned		     comp_cnt;	/**< Number of components.	*/
@@ -985,6 +987,12 @@ static void ice_st_on_destroy(void *obj)
 {
     pj_ice_strans *ice_st = (pj_ice_strans*)obj;
 
+    /* Destroy any previous ICE session */
+    if (ice_st->ice_prev) {
+	(*ice_st->ice_prev_hndlr)(ice_st->ice_prev);
+	ice_st->ice_prev = NULL;
+    }
+
     PJ_LOG(4,(ice_st->obj_name, "ICE stream transport %p destroyed", obj));
 
     /* Done */
@@ -1016,8 +1024,9 @@ static void destroy_ice_st(pj_ice_strans *ice_st)
 
     /* Destroy ICE if we have ICE */
     if (ice_st->ice) {
-	pj_ice_sess_destroy(ice_st->ice);
+	pj_ice_sess *ice = ice_st->ice;
 	ice_st->ice = NULL;
+	pj_ice_sess_destroy(ice);
     }
 
     /* Destroy all components */
@@ -1279,6 +1288,15 @@ PJ_DEF(pj_status_t) pj_ice_strans_init_ice(pj_ice_strans *ice_st,
     ice_cb.on_rx_data = &ice_rx_data;
     ice_cb.on_tx_pkt = &ice_tx_pkt;
 
+    /* Release the pool of previous ICE session to avoid memory bloat,
+     * as otherwise it will only be released after ICE strans is destroyed
+     * (due to group lock).
+     */
+    if (ice_st->ice_prev) {
+	(*ice_st->ice_prev_hndlr)(ice_st->ice_prev);
+	ice_st->ice_prev = NULL;
+    }
+
     /* Create! */
     status = pj_ice_sess_create(&ice_st->cfg.stun_cfg, ice_st->obj_name, role,
 			        ice_st->comp_cnt, &ice_cb,
@@ -1726,8 +1744,10 @@ PJ_DEF(pj_status_t) pj_ice_strans_stop_ice(pj_ice_strans *ice_st)
     pj_grp_lock_acquire(ice_st->grp_lock);
 
     if (ice_st->ice) {
-	pj_ice_sess_destroy(ice_st->ice);
+	ice_st->ice_prev = ice_st->ice;
 	ice_st->ice = NULL;
+	pj_ice_sess_detach_grp_lock(ice_st->ice_prev, &ice_st->ice_prev_hndlr);
+	pj_ice_sess_destroy(ice_st->ice_prev);
     }
 
     ice_st->state = PJ_ICE_STRANS_STATE_INIT;