Changeset 4530

Timestamp:

May 30, 2013 9:27:49 AM (11 years ago)

Author:

nanang

Message:

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.

Location:

pjproject/trunk/pjsip

Files:

• include/pjsip/sip_config.h (modified) (1 diff)
• src/pjsip-simple/publishc.c (modified) (6 diffs)
• src/pjsip/sip_transport.c (modified) (7 diffs)

pjproject/trunk/pjsip/include/pjsip/sip_config.h

@@ -1133 +1133 @@
 
 
+/**
+ * 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
 

pjproject/trunk/pjsip/src/pjsip-simple/publishc.c

@@ -62 +62 @@
 typedef struct pending_publish
 {
-    PJ_DECL_LIST_MEMBER(pjsip_tx_data);
+    PJ_DECL_LIST_MEMBER(struct pending_publish);
+    pjsip_tx_data               *tdata;
 } pending_publish;
 
@@ -109 +110 @@
     /* Pending PUBLISH request */
     pending_publish              pending_reqs;
+    pending_publish              pending_reqs_empty;
 };
 
@@ -181 +183 @@
     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);
@@ -684 +687 @@
         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
@@ -713 +722 @@
             if (status == PJ_EPENDING) {
                 pj_assert(!"Not expected");
-                pj_list_erase(tdata);
                 pjsip_tx_data_dec_ref(tdata);
             } else if (status == PJ_SUCCESS) {
@@ -745 +753 @@
     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 "

pjproject/trunk/pjsip/src/pjsip/sip_transport.c

@@ -93 +93 @@
     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;
 };
 
@@ -420 +425 @@
 
     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
@@ -438 +451 @@
 }
 
+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
@@ -446 +480 @@
     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 {
@@ -1208 +1234 @@
     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);
@@ -1498 +1525 @@
 
     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
@@ -1518 +1539 @@
     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;