Ticket #877: Memory consumption of the invite session grows indefinitely if call is running for long period of time and with many re-INVITES

 - introducing flip-flop pools in the pjsip_inv_session. There are two additional pools created, and one of them will be reset everytime SDP negotiation is done to release memory back to the OS


git-svn-id: http://svn.pjsip.org/repos/pjproject/trunk@2869 74dad513-b988-da41-8d7b-12977e46ad98

5 files changed, 109 insertions, 34 deletions

diff --git a/pjsip/include/pjsip-ua/sip_inv.h b/pjsip/include/pjsip-ua/sip_inv.h
index e85b00b0..a2281e47 100644
--- a/pjsip/include/pjsip-ua/sip_inv.h
+++ b/pjsip/include/pjsip-ua/sip_inv.h
@@ -318,11 +318,39 @@ struct pjsip_timer;
 
 /**
  * This structure describes the invite session.
+ *
+ * Note regarding the invite session's pools. The inv_sess used to have
+ * only one pool, which is just a pointer to the dialog's pool. Ticket
+ * http://trac.pjsip.org/repos/ticket/877 has found that the memory
+ * usage will grow considerably everytime re-INVITE or UPDATE is
+ * performed.
+ *
+ * Ticket #877 then created two more memory pools for the inv_sess, so
+ * now we have three memory pools:
+ *  - pool: to be used to allocate long term data for the session
+ *  - pool_prov and pool_active: this is a flip-flop pools to be used
+ *     interchangably during re-INVITE and UPDATE. pool_prov is
+ *     "provisional" pool, used to allocate SDP offer or answer for
+ *     the re-INVITE and UPDATE. Once SDP negotiation is done, the
+ *     provisional pool will be made as the active pool, then the
+ *     existing active pool will be reset, to release the memory
+ *     back to the OS. So these pool's lifetime is synchronized to
+ *     the SDP offer-answer negotiation.
+ *
+ * Higher level application such as PJSUA-LIB has been modified to
+ * make use of these flip-flop pools, i.e. by creating media objects
+ * from the provisional pool rather than from the long term pool.
+ *
+ * Other applications that want to use these pools must understand
+ * that the flip-flop pool's lifetimes are synchronized to the
+ * SDP offer-answer negotiation.
  */
 struct pjsip_inv_session
 {
     char		 obj_name[PJ_MAX_OBJ_NAME]; /**< Log identification */
-    pj_pool_t		*pool;			    /**< Dialog's pool.	    */
+    pj_pool_t		*pool;			    /**< Long term pool.    */
+    pj_pool_t		*pool_prov;		    /**< Provisional pool   */
+    pj_pool_t		*pool_active;		    /**< Active/current pool*/
     pjsip_inv_state	 state;			    /**< Invite sess state. */
     pj_bool_t		 cancelling;		    /**< CANCEL requested   */
     pj_bool_t		 pending_cancel;	    /**< Wait to send CANCEL*/
diff --git a/pjsip/src/pjsip-ua/sip_inv.c b/pjsip/src/pjsip-ua/sip_inv.c
index 53858dcd..e7eb1c58 100644
--- a/pjsip/src/pjsip-ua/sip_inv.c
+++ b/pjsip/src/pjsip-ua/sip_inv.c
@@ -77,6 +77,9 @@ static const pjsip_method pjsip_update_method =
     { "UPDATE", 6 }
 };
 
+#define POOL_INIT_SIZE	256
+#define POOL_INC_SIZE	256
+
 /*
  * Static prototypes.
  */
@@ -234,6 +237,10 @@ void inv_set_state(pjsip_inv_session *inv, pjsip_inv_state state,
 	pjsip_100rel_end_session(inv);
 	pjsip_timer_end_session(inv);
 	pjsip_dlg_dec_session(inv->dlg, &mod_inv.mod);
+
+	/* Release the flip-flop pools */
+	pj_pool_release(inv->pool_prov);
+	pj_pool_release(inv->pool_active);
     }
 }
 
@@ -669,13 +676,21 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uac( pjsip_dialog *dlg,
     inv->notify = PJ_TRUE;
     inv->cause = (pjsip_status_code) 0;
 
+    /* Create flip-flop pool (see ticket #877) */
+    /* (using inv->obj_name as temporary variable for pool names */
+    pj_ansi_snprintf(inv->obj_name, PJ_MAX_OBJ_NAME, "inv%p", dlg->pool);
+    inv->pool_prov = pjsip_endpt_create_pool(dlg->endpt, inv->obj_name,
+					     POOL_INIT_SIZE, POOL_INC_SIZE);
+    inv->pool_active = pjsip_endpt_create_pool(dlg->endpt, inv->obj_name,
+					       POOL_INIT_SIZE, POOL_INC_SIZE);
+
     /* Object name will use the same dialog pointer. */
     pj_ansi_snprintf(inv->obj_name, PJ_MAX_OBJ_NAME, "inv%p", dlg);
 
     /* Create negotiator if local_sdp is specified. */
     if (local_sdp) {
-	status = pjmedia_sdp_neg_create_w_local_offer(dlg->pool, local_sdp,
-						      &inv->neg);
+	status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, 
+						      local_sdp, &inv->neg);
 	if (status != PJ_SUCCESS) {
 	    pjsip_dlg_dec_lock(dlg);
 	    return status;
@@ -1129,6 +1144,14 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uas( pjsip_dialog *dlg,
     inv->notify = PJ_TRUE;
     inv->cause = (pjsip_status_code) 0;
 
+    /* Create flip-flop pool (see ticket #877) */
+    /* (using inv->obj_name as temporary variable for pool names */
+    pj_ansi_snprintf(inv->obj_name, PJ_MAX_OBJ_NAME, "inv%p", dlg->pool);
+    inv->pool_prov = pjsip_endpt_create_pool(dlg->endpt, inv->obj_name,
+					     POOL_INIT_SIZE, POOL_INC_SIZE);
+    inv->pool_active = pjsip_endpt_create_pool(dlg->endpt, inv->obj_name,
+					       POOL_INIT_SIZE, POOL_INC_SIZE);
+
     /* Object name will use the same dialog pointer. */
     pj_ansi_snprintf(inv->obj_name, PJ_MAX_OBJ_NAME, "inv%p", dlg);
 
@@ -1150,12 +1173,13 @@ PJ_DEF(pj_status_t) pjsip_inv_create_uas( pjsip_dialog *dlg,
 
     /* Create negotiator. */
     if (rem_sdp) {
-	status = pjmedia_sdp_neg_create_w_remote_offer(inv->pool, local_sdp,
-						       rem_sdp, &inv->neg);
+	status = pjmedia_sdp_neg_create_w_remote_offer(inv->pool, 
+						       local_sdp, rem_sdp, 
+						       &inv->neg);
 						
     } else if (local_sdp) {
-	status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, local_sdp,
-						      &inv->neg);
+	status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, 
+						      local_sdp, &inv->neg);
     } else {
 	status = PJ_SUCCESS;
     }
@@ -1439,6 +1463,14 @@ on_return:
 }
 
 
+/* Util: swap pool */
+static void swap_pool(pj_pool_t **p1, pj_pool_t **p2)
+{
+    pj_pool_t *tmp = *p1;
+    *p1 = *p2;
+    *p2 = tmp;
+}
+
 /*
  * Initiate SDP negotiation in the SDP negotiator.
  */
@@ -1450,13 +1482,17 @@ static pj_status_t inv_negotiate_sdp( pjsip_inv_session *inv )
 		     PJMEDIA_SDP_NEG_STATE_WAIT_NEGO, 
 		     PJMEDIA_SDPNEG_EINSTATE);
 
-    status = pjmedia_sdp_neg_negotiate(inv->pool, inv->neg, 0);
+    status = pjmedia_sdp_neg_negotiate(inv->pool_prov, inv->neg, 0);
 
     PJ_LOG(5,(inv->obj_name, "SDP negotiation done, status=%d", status));
 
     if (mod_inv.cb.on_media_update && inv->notify)
 	(*mod_inv.cb.on_media_update)(inv, status);
 
+    /* Swap the flip-flop pool, and reset the new provisional pool */
+    swap_pool(&inv->pool_prov, &inv->pool_active);
+    pj_pool_reset(inv->pool_prov);
+
     return status;
 }
 
@@ -1534,7 +1570,8 @@ static pj_status_t inv_check_sdp_in_incoming_msg( pjsip_inv_session *inv,
 			  tsx->last_tx->msg->body->data;
 
 	    /* Feed the original offer to negotiator */
-	    status = pjmedia_sdp_neg_modify_local_offer(inv->pool, inv->neg,
+	    status = pjmedia_sdp_neg_modify_local_offer(inv->pool_prov, 
+							inv->neg,
 						        reoffer_sdp);
 	    if (status != PJ_SUCCESS) {
 		PJ_LOG(1,(inv->obj_name, "Error updating local offer for "
@@ -1580,10 +1617,10 @@ static pj_status_t inv_check_sdp_in_incoming_msg( pjsip_inv_session *inv,
 		  pjsip_rx_data_get_info(rdata)));
 
 	if (inv->neg == NULL) {
-	    status=pjmedia_sdp_neg_create_w_remote_offer(inv->pool, NULL, 
+	    status=pjmedia_sdp_neg_create_w_remote_offer(inv->pool, NULL,
 							 rem_sdp, &inv->neg);
 	} else {
-	    status=pjmedia_sdp_neg_set_remote_offer(inv->pool, inv->neg, 
+	    status=pjmedia_sdp_neg_set_remote_offer(inv->pool_prov, inv->neg, 
 						    rem_sdp);
 	}
 
@@ -1615,7 +1652,7 @@ static pj_status_t inv_check_sdp_in_incoming_msg( pjsip_inv_session *inv,
 	PJ_LOG(5,(inv->obj_name, "Got SDP answer in %s", 
 		  pjsip_rx_data_get_info(rdata)));
 
-	status = pjmedia_sdp_neg_set_remote_answer(inv->pool, inv->neg, 
+	status = pjmedia_sdp_neg_set_remote_answer(inv->pool_prov, inv->neg,
 						   rem_sdp);
 
 	if (status != PJ_SUCCESS) {
@@ -1668,12 +1705,13 @@ static pj_status_t process_answer( pjsip_inv_session *inv,
     if (local_sdp && (st_code/100==1 || st_code/100==2)) {
 
 	if (inv->neg == NULL) {
-	    status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, local_sdp,
+	    status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, 
+							  local_sdp,
 							  &inv->neg);
 	} else if (pjmedia_sdp_neg_get_state(inv->neg)==
 		   PJMEDIA_SDP_NEG_STATE_REMOTE_OFFER)
 	{
-	    status = pjmedia_sdp_neg_set_local_answer(inv->pool, inv->neg,
+	    status = pjmedia_sdp_neg_set_local_answer(inv->pool_prov, inv->neg,
 						      local_sdp);
 	} else {
 
@@ -1878,7 +1916,7 @@ PJ_DEF(pj_status_t) pjsip_inv_set_sdp_answer( pjsip_inv_session *inv,
     PJ_ASSERT_RETURN(inv && sdp, PJ_EINVAL);
 
     pjsip_dlg_inc_lock(inv->dlg);
-    status = pjmedia_sdp_neg_set_local_answer( inv->pool, inv->neg, sdp);
+    status = pjmedia_sdp_neg_set_local_answer( inv->pool_prov, inv->neg, sdp);
     pjsip_dlg_dec_lock(inv->dlg);
 
     return status;
@@ -2227,7 +2265,8 @@ PJ_DEF(pj_status_t) pjsip_inv_reinvite( pjsip_inv_session *inv,
 
     if (new_offer) {
 	if (!inv->neg) {
-	    status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, new_offer,
+	    status = pjmedia_sdp_neg_create_w_local_offer(inv->pool, 
+							  new_offer,
 							  &inv->neg);
 	    if (status != PJ_SUCCESS)
 		goto on_return;
@@ -2246,7 +2285,8 @@ PJ_DEF(pj_status_t) pjsip_inv_reinvite( pjsip_inv_session *inv,
 		break;
 
 	    case PJMEDIA_SDP_NEG_STATE_REMOTE_OFFER:
-		status = pjmedia_sdp_neg_set_local_answer(inv->pool, inv->neg,
+		status = pjmedia_sdp_neg_set_local_answer(inv->pool_prov, 
+							  inv->neg,
 							  new_offer);
 		if (status != PJ_SUCCESS)
 		    goto on_return;
@@ -2259,7 +2299,8 @@ PJ_DEF(pj_status_t) pjsip_inv_reinvite( pjsip_inv_session *inv,
 		break;
 
 	    case PJMEDIA_SDP_NEG_STATE_DONE:
-		status = pjmedia_sdp_neg_modify_local_offer(inv->pool,inv->neg,
+		status = pjmedia_sdp_neg_modify_local_offer(inv->pool_prov,
+							    inv->neg,
 							    new_offer);
 		if (status != PJ_SUCCESS)
 		    goto on_return;
@@ -2315,7 +2356,7 @@ PJ_DEF(pj_status_t) pjsip_inv_update (	pjsip_inv_session *inv,
     /* Notify negotiator about the new offer. This will fix the offer
      * with correct SDP origin.
      */
-    status = pjmedia_sdp_neg_modify_local_offer(inv->pool,inv->neg,
+    status = pjmedia_sdp_neg_modify_local_offer(inv->pool_prov, inv->neg,
 						offer);
     if (status != PJ_SUCCESS)
 	goto on_error;
@@ -3744,9 +3785,10 @@ static void inv_on_state_confirmed( pjsip_inv_session *inv, pjsip_event *e)
 			/* Notify negotiator about the new offer. This will
 			 * fix the offer with correct SDP origin.
 			 */
-			status = pjmedia_sdp_neg_modify_local_offer(dlg->pool,
-								    inv->neg,
-								    sdp);
+			status = 
+			    pjmedia_sdp_neg_modify_local_offer(inv->pool_prov,
+							       inv->neg,
+							       sdp);
 
 			/* Retrieve the "fixed" offer from negotiator */
 			if (status==PJ_SUCCESS) {
@@ -3759,7 +3801,7 @@ static void inv_on_state_confirmed( pjsip_inv_session *inv, pjsip_event *e)
 		
 		if (sdp == NULL) {
 		    const pjmedia_sdp_session *active_sdp = NULL;
-		    status = pjmedia_sdp_neg_send_local_offer(dlg->pool, 
+		    status = pjmedia_sdp_neg_send_local_offer(inv->pool_prov,
 							      inv->neg, 
 							      &active_sdp);
 		    if (status == PJ_SUCCESS)
diff --git a/pjsip/src/pjsip-ua/sip_timer.c b/pjsip/src/pjsip-ua/sip_timer.c
index 36209448..3c60e01d 100644
--- a/pjsip/src/pjsip-ua/sip_timer.c
+++ b/pjsip/src/pjsip-ua/sip_timer.c
@@ -359,7 +359,7 @@ void timer_cb(pj_timer_heap_t *timer_heap, struct pj_timer_entry *entry)
 
 	    status = pjsip_inv_invite(inv, &tdata);
 	    if (status == PJ_SUCCESS)
-		status = pjmedia_sdp_neg_send_local_offer(inv->pool, 
+		status = pjmedia_sdp_neg_send_local_offer(inv->pool_prov, 
 							  inv->neg, &offer);
 	    if (status == PJ_SUCCESS)
 		status = pjmedia_sdp_neg_get_neg_local(inv->neg, &offer);
diff --git a/pjsip/src/pjsua-lib/pjsua_call.c b/pjsip/src/pjsua-lib/pjsua_call.c
index e244aec0..5c493d03 100644
--- a/pjsip/src/pjsua-lib/pjsua_call.c
+++ b/pjsip/src/pjsua-lib/pjsua_call.c
@@ -1596,7 +1596,8 @@ PJ_DEF(pj_status_t) pjsua_call_reinvite( pjsua_call_id call_id,
     if (call->local_hold && !unhold) {
 	status = create_sdp_of_call_hold(call, &sdp);
     } else {
-	status = pjsua_media_channel_create_sdp(call->index, call->inv->pool, 
+	status = pjsua_media_channel_create_sdp(call->index, 
+						call->inv->pool_prov,
 						NULL, &sdp, NULL);
 	call->local_hold = PJ_FALSE;
     }
@@ -1655,7 +1656,8 @@ PJ_DEF(pj_status_t) pjsua_call_update( pjsua_call_id call_id,
 	return status;
 
     /* Create SDP */
-    status = pjsua_media_channel_create_sdp(call->index, call->inv->pool, 
+    status = pjsua_media_channel_create_sdp(call->index, 
+					    call->inv->pool_prov, 
 					    NULL, &sdp, NULL);
     if (status != PJ_SUCCESS) {
 	pjsua_perror(THIS_FILE, "Unable to get SDP from media endpoint", 
@@ -3200,8 +3202,8 @@ static pj_status_t create_sdp_of_call_hold(pjsua_call *call,
     pj_pool_t *pool;
     pjmedia_sdp_session *sdp;
 
-    /* Use call's pool */
-    pool = call->inv->pool;
+    /* Use call's provisional pool */
+    pool = call->inv->pool_prov;
 
     /* Create new offer */
     status = pjsua_media_channel_create_sdp(call->index, pool, NULL, &sdp, 
@@ -3267,7 +3269,8 @@ static void pjsua_call_on_rx_offer(pjsip_inv_session *inv,
     PJ_LOG(4,(THIS_FILE, "Call %d: received updated media offer",
 	      call->index));
 
-    status = pjsua_media_channel_create_sdp(call->index, call->inv->pool, 
+    status = pjsua_media_channel_create_sdp(call->index, 
+					    call->inv->pool_prov, 
 					    offer, &answer, NULL);
     if (status != PJ_SUCCESS) {
 	pjsua_perror(THIS_FILE, "Unable to create local SDP", status);
@@ -3296,7 +3299,7 @@ static void pjsua_call_on_rx_offer(pjsip_inv_session *inv,
 	/* Keep call on-hold by setting 'sendonly' attribute.
 	 * (See RFC 3264 Section 8.4 and RFC 4317 Section 3.1)
 	 */
-	attr = pjmedia_sdp_attr_create(call->inv->pool, "sendonly", NULL);
+	attr = pjmedia_sdp_attr_create(call->inv->pool_prov, "sendonly", NULL);
 	pjmedia_sdp_media_add_attr(answer->media[0], attr);
     }
 
@@ -3334,7 +3337,8 @@ static void pjsua_call_on_create_offer(pjsip_inv_session *inv,
 	PJ_LOG(4,(THIS_FILE, "Call %d: asked to send a new offer",
 		  call->index));
 
-	status = pjsua_media_channel_create_sdp(call->index, call->inv->pool, 
+	status = pjsua_media_channel_create_sdp(call->index, 
+						call->inv->pool_prov, 
 					        NULL, offer, NULL);
     }
 
diff --git a/pjsip/src/pjsua-lib/pjsua_media.c b/pjsip/src/pjsua-lib/pjsua_media.c
index 7b95a137..243f4f65 100644
--- a/pjsip/src/pjsua-lib/pjsua_media.c
+++ b/pjsip/src/pjsua-lib/pjsua_media.c
@@ -1426,7 +1426,7 @@ pj_status_t pjsua_media_channel_update(pjsua_call_id call_id,
 
     /* Create media session info based on SDP parameters. 
      */    
-    status = pjmedia_session_info_from_sdp( call->inv->dlg->pool, 
+    status = pjmedia_session_info_from_sdp( call->inv->pool_prov, 
 					    pjsua_var.med_endpt, 
 					    PJMEDIA_MAX_SDP_MEDIA, &sess_info,
 					    local_sdp, remote_sdp);
@@ -1469,7 +1469,7 @@ pj_status_t pjsua_media_channel_update(pjsua_call_id call_id,
 
 	/* Start/restart media transport */
 	status = pjmedia_transport_media_start(call->med_tp, 
-					       call->inv->pool,
+					       call->inv->pool_prov,
 					       local_sdp, remote_sdp, 0);
 	if (status != PJ_SUCCESS)
 	    return status;
@@ -1572,7 +1572,8 @@ pj_status_t pjsua_media_channel_update(pjsua_call_id call_id,
 	    if (port_name.slen < 1) {
 		port_name = pj_str("call");
 	    }
-	    status = pjmedia_conf_add_port( pjsua_var.mconf, call->inv->pool,
+	    status = pjmedia_conf_add_port( pjsua_var.mconf, 
+					    call->inv->pool_prov,
 					    media_port, 
 					    &port_name,
 					    (unsigned*)&call->conf_slot);