Fixed #1671:

 - Transport manager maintains transmit buffer instance list, so any dangling transmit buffer will be freed when transport manager is destroyed. This is configurable via PJSIP_HAS_TX_DATA_LIST, the default is zero/disabled.
 - Updated publish client subscription to not use the 'internal' pjsip_tx_data list structure.



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

3 files changed, 92 insertions, 20 deletions

diff --git a/pjsip/include/pjsip/sip_config.h b/pjsip/include/pjsip/sip_config.h
index 430dde22..ec991e88 100644
--- a/pjsip/include/pjsip/sip_config.h
+++ b/pjsip/include/pjsip/sip_config.h
@@ -1132,6 +1132,21 @@ PJ_INLINE(pjsip_cfg_t*) pjsip_cfg(void)
 #endif
 
 
+/**
+ * Specify whether transport manager should maintain a list of transmit
+ * buffer instances, so any possible dangling instance can be cleaned up
+ * when the transport manager is shutdown (see also ticket #1671).
+ * Note that this feature will have slight impact on the performance as
+ * mutex is employed in updating the list, i.e: on creation and destruction
+ * of transmit data.
+ *
+ * Default: 0 (no)
+ */
+#ifndef PJSIP_HAS_TX_DATA_LIST
+#   define PJSIP_HAS_TX_DATA_LIST		0
+#endif
+
+
 PJ_END_DECL
 
 /**
diff --git a/pjsip/src/pjsip-simple/publishc.c b/pjsip/src/pjsip-simple/publishc.c
index af8d0b3e..fcb412dd 100644
--- a/pjsip/src/pjsip-simple/publishc.c
+++ b/pjsip/src/pjsip-simple/publishc.c
@@ -61,7 +61,8 @@ const pjsip_method pjsip_publish_method =
  */
 typedef struct pending_publish
 {
-    PJ_DECL_LIST_MEMBER(pjsip_tx_data);
+    PJ_DECL_LIST_MEMBER(struct pending_publish);
+    pjsip_tx_data		*tdata;
 } pending_publish;
 
 
@@ -108,6 +109,7 @@ struct pjsip_publishc
 
     /* Pending PUBLISH request */
     pending_publish		 pending_reqs;
+    pending_publish		 pending_reqs_empty;
 };
 
 
@@ -180,6 +182,7 @@ PJ_DEF(pj_status_t) pjsip_publishc_create( pjsip_endpoint *endpt,
     }
     pj_memcpy(&pubc->opt, opt, sizeof(*opt));
     pj_list_init(&pubc->pending_reqs);
+    pj_list_init(&pubc->pending_reqs_empty);
 
     status = pj_mutex_create_recursive(pubc->pool, "pubc%p", &pubc->mutex);
     if (status != PJ_SUCCESS) {
@@ -683,8 +686,14 @@ static void tsx_callback(void *token, pjsip_event *event)
 	/* If we have pending request(s), send them now */
 	pj_mutex_lock(pubc->mutex);
 	while (!pj_list_empty(&pubc->pending_reqs)) {
-	    pjsip_tx_data *tdata = pubc->pending_reqs.next;
-	    pj_list_erase(tdata);
+	    pending_publish *pp = pubc->pending_reqs.next;
+	    pjsip_tx_data *tdata = pp->tdata;
+
+	    /* Remove the request from pending request list,
+	     * and keep the unused entry into pending_reqs_empty pool.
+	     */
+	    pj_list_erase(pp);
+	    pj_list_push_back(&pubc->pending_reqs_empty, pp);
 
 	    /* Add SIP-If-Match if we have etag and the request doesn't have
 	     * one (http://trac.pjsip.org/repos/ticket/996)
@@ -712,7 +721,6 @@ static void tsx_callback(void *token, pjsip_event *event)
 	    status = pjsip_publishc_send(pubc, tdata);
 	    if (status == PJ_EPENDING) {
 		pj_assert(!"Not expected");
-		pj_list_erase(tdata);
 		pjsip_tx_data_dec_ref(tdata);
 	    } else if (status == PJ_SUCCESS) {
 		break;
@@ -744,7 +752,15 @@ PJ_DEF(pj_status_t) pjsip_publishc_send(pjsip_publishc *pubc,
     pj_mutex_lock(pubc->mutex);
     if (pubc->pending_tsx) {
 	if (pubc->opt.queue_request) {
-	    pj_list_push_back(&pubc->pending_reqs, tdata);
+	    pending_publish *pp = NULL;
+	    if (pj_list_empty(&pubc->pending_reqs_empty)) {
+		pp = PJ_POOL_ZALLOC_T(pubc->pool, pending_publish);
+	    } else {
+		pp = pubc->pending_reqs_empty.next;
+		pj_list_erase(pp);
+	    }
+	    pp->tdata = tdata;
+	    pj_list_push_back(&pubc->pending_reqs, pp);
 	    pj_mutex_unlock(pubc->mutex);
 	    PJ_LOG(4,(THIS_FILE, "Request is queued, pubc has another "
 				 "transaction pending"));
diff --git a/pjsip/src/pjsip/sip_transport.c b/pjsip/src/pjsip/sip_transport.c
index ef2de49e..d1ee6178 100644
--- a/pjsip/src/pjsip/sip_transport.c
+++ b/pjsip/src/pjsip/sip_transport.c
@@ -92,6 +92,11 @@ struct pjsip_tpmgr
     void           (*on_rx_msg)(pjsip_endpoint*, pj_status_t, pjsip_rx_data*);
     pj_status_t	   (*on_tx_msg)(pjsip_endpoint*, pjsip_tx_data*);
     pjsip_tp_state_callback tp_state_cb;
+
+    /* Transmit data list, for transmit data cleanup when transport manager
+     * is destroyed.
+     */
+    pjsip_tx_data    tdata_list;
 };
 
 
@@ -419,6 +424,14 @@ PJ_DEF(pj_status_t) pjsip_tx_data_create( pjsip_tpmgr *mgr,
     }
 
     pj_ioqueue_op_key_init(&tdata->op_key.key, sizeof(tdata->op_key.key));
+    pj_list_init(tdata);
+
+#if defined(PJSIP_HAS_TX_DATA_LIST) && PJSIP_HAS_TX_DATA_LIST!=0
+    /* Append this just created tdata to transmit buffer list */
+    pj_lock_acquire(mgr->lock);
+    pj_list_push_back(&mgr->tdata_list, tdata);
+    pj_lock_release(mgr->lock);
+#endif
 
 #if defined(PJ_DEBUG) && PJ_DEBUG!=0
     pj_atomic_inc( tdata->mgr->tdata_counter );
@@ -437,6 +450,27 @@ PJ_DEF(void) pjsip_tx_data_add_ref( pjsip_tx_data *tdata )
     pj_atomic_inc(tdata->ref_cnt);
 }
 
+static void tx_data_destroy(pjsip_tx_data *tdata)
+{
+    PJ_LOG(5,(tdata->obj_name, "Destroying txdata %s",
+	      pjsip_tx_data_get_info(tdata)));
+    pjsip_tpselector_dec_ref(&tdata->tp_sel);
+#if defined(PJ_DEBUG) && PJ_DEBUG!=0
+    pj_atomic_dec( tdata->mgr->tdata_counter );
+#endif
+
+#if defined(PJSIP_HAS_TX_DATA_LIST) && PJSIP_HAS_TX_DATA_LIST!=0
+    /* Remove this tdata from transmit buffer list */
+    pj_lock_acquire(tdata->mgr->lock);
+    pj_list_erase(tdata);
+    pj_lock_release(tdata->mgr->lock);
+#endif
+
+    pj_atomic_destroy( tdata->ref_cnt );
+    pj_lock_destroy( tdata->lock );
+    pjsip_endpt_release_pool( tdata->mgr->endpt, tdata->pool );
+}
+
 /*
  * Decrease transport data reference, destroy it when the reference count
  * reaches zero.
@@ -445,15 +479,7 @@ PJ_DEF(pj_status_t) pjsip_tx_data_dec_ref( pjsip_tx_data *tdata )
 {
     pj_assert( pj_atomic_get(tdata->ref_cnt) > 0);
     if (pj_atomic_dec_and_get(tdata->ref_cnt) <= 0) {
-	PJ_LOG(5,(tdata->obj_name, "Destroying txdata %s",
-		  pjsip_tx_data_get_info(tdata)));
-	pjsip_tpselector_dec_ref(&tdata->tp_sel);
-#if defined(PJ_DEBUG) && PJ_DEBUG!=0
-	pj_atomic_dec( tdata->mgr->tdata_counter );
-#endif
-	pj_atomic_destroy( tdata->ref_cnt );
-	pj_lock_destroy( tdata->lock );
-	pjsip_endpt_release_pool( tdata->mgr->endpt, tdata->pool );
+	tx_data_destroy(tdata);
 	return PJSIP_EBUFDESTROYED;
     } else {
 	return PJ_SUCCESS;
@@ -1207,6 +1233,7 @@ PJ_DEF(pj_status_t) pjsip_tpmgr_create( pj_pool_t *pool,
     mgr->on_rx_msg = rx_cb;
     mgr->on_tx_msg = tx_cb;
     pj_list_init(&mgr->factory_list);
+    pj_list_init(&mgr->tdata_list);
 
     mgr->table = pj_hash_create(pool, PJSIP_TPMGR_HTABLE_SIZE);
     if (!mgr->table)
@@ -1497,12 +1524,6 @@ PJ_DEF(pj_status_t) pjsip_tpmgr_destroy( pjsip_tpmgr *mgr )
     }
 
     pj_lock_release(mgr->lock);
-    pj_lock_destroy(mgr->lock);
-
-    /* Unregister mod_msg_print. */
-    if (mod_msg_print.id != -1) {
-	pjsip_endpt_unregister_module(endpt, &mod_msg_print);
-    }
 
 #if defined(PJ_DEBUG) && PJ_DEBUG!=0
     /* If you encounter assert error on this line, it means there are
@@ -1518,6 +1539,26 @@ PJ_DEF(pj_status_t) pjsip_tpmgr_destroy( pjsip_tpmgr *mgr )
     pj_atomic_destroy(mgr->tdata_counter);
 #endif
 
+    /*
+     * Destroy any dangling transmit buffer.
+     */
+    if (!pj_list_empty(&mgr->tdata_list)) {
+	pjsip_tx_data *tdata = mgr->tdata_list.next;
+	while (tdata != &mgr->tdata_list) {
+	    pjsip_tx_data *next = tdata->next;
+	    tx_data_destroy(tdata);
+	    tdata = next;
+	}
+	PJ_LOG(3,(THIS_FILE, "Cleaned up dangling transmit buffer(s)."));
+    }
+
+    pj_lock_destroy(mgr->lock);
+
+    /* Unregister mod_msg_print. */
+    if (mod_msg_print.id != -1) {
+	pjsip_endpt_unregister_module(endpt, &mod_msg_print);
+    }
+
     return PJ_SUCCESS;
 }