Changeset 5246

Timestamp:

Feb 25, 2016 4:38:34 AM (9 years ago)

Author:

nanang

Message:

Fix #1311: Updated pjsip_tpmgr_acquire_transport2() to look up from transport hash table (instead of always create a new one) when transport selector is set to TCP/TLS listener (thanks George Joseph for the patch).

Location:

pjproject/trunk/pjsip

Files:

• include/pjsip/sip_transport.h (modified) (1 diff)
• src/pjsip/sip_transport.c (modified) (6 diffs)

pjproject/trunk/pjsip/include/pjsip/sip_transport.h

@@ -800 +800 @@
     pjsip_endpoint         *endpt;          /**< Endpoint instance.         */
     pjsip_tpmgr            *tpmgr;          /**< Transport manager.         */
+    pjsip_tpfactory        *factory;        /**< Factory instance. Note: it
+                                                 may be invalid/shutdown.   */
     pj_timer_entry          idle_timer;     /**< Timer when ref cnt is zero.*/
 

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

@@ -1296 +1296 @@
 }
 
+static pj_bool_t pjsip_tpmgr_is_tpfactory_valid(pjsip_tpmgr *mgr,
+                                                pjsip_tpfactory *tpf)
+{
+    pjsip_tpfactory *p;
+
+    pj_lock_acquire(mgr->lock);
+    for (p=mgr->factory_list.next; p!=&mgr->factory_list; p=p->next) {
+        if (p == tpf) {
+            pj_lock_release(mgr->lock);
+            return PJ_TRUE;
+        }
+    }
+    pj_lock_release(mgr->lock);
+
+    return PJ_FALSE;
+}
+
 /*****************************************************************************
  *
@@ -2001 +2018 @@
         return PJ_SUCCESS;
 
-
-    } else if (sel && sel->type == PJSIP_TPSELECTOR_LISTENER &&
-               sel->u.listener)
-    {
-        /* Application has requested that a specific listener is to
-         * be used. In this case, skip transport hash table lookup.
-         */
-
-        /* Verify that the listener type matches the destination type */
-        if (sel->u.listener->type != type) {
-            pj_lock_release(mgr->lock);
-            return PJSIP_ETPNOTSUITABLE;
-        }
-
-        /* We'll use this listener to create transport */
-        factory = sel->u.listener;
-
     } else {
 
         /*
          * This is the "normal" flow, where application doesn't specify
-         * specific transport/listener to be used to send message to.
+         * specific transport to be used to send message to.
          * In this case, lookup the transport from the hash table.
          */
@@ -2028 +2028 @@
         int key_len;
         pjsip_transport *transport;
+
+        /* If listener is specified, verify that the listener type matches
+         * the destination type.
+         */
+        if (sel && sel->type == PJSIP_TPSELECTOR_LISTENER && sel->u.listener)
+        {
+            if (sel->u.listener->type != type) {
+                pj_lock_release(mgr->lock);
+                return PJSIP_ETPNOTSUITABLE;
+            }
+        }
 
         pj_bzero(&key, sizeof(key));
@@ -2070 +2081 @@
         }
 
+        /* If transport is found and listener is specified, verify listener */
+        else if (sel && sel->type == PJSIP_TPSELECTOR_LISTENER &&
+                 sel->u.listener && transport->factory != sel->u.listener)
+        {
+            transport = NULL;
+            /* This will cause a new transport to be created which will be a
+             * 'duplicate' of the existing transport (same type & remote addr,
+             * but different factory). Any future hash lookup will return
+             * the new one, and eventually the old one will still be freed
+             * (by application or #1774).
+             */
+        }
+
         if (transport!=NULL && !transport->is_shutdown) {
             /*
@@ -2082 +2106 @@
         }
 
+
         /*
          * Transport not found!
-         * Find factory that can create such transport.
+         * So we need to create one, find factory that can create
+         * such transport.
          */
-        factory = mgr->factory_list.next;
-        while (factory != &mgr->factory_list) {
-            if (factory->type == type)
-                break;
-            factory = factory->next;
-        }
-
-        if (factory == &mgr->factory_list) {
-            /* No factory can create the transport! */
-            pj_lock_release(mgr->lock);
-            TRACE_((THIS_FILE, "No suitable factory was found either"));
-            return PJSIP_EUNSUPTRANSPORT;
+        if (sel && sel->type == PJSIP_TPSELECTOR_LISTENER && sel->u.listener)
+        {
+            /* Application has requested that a specific listener is to
+             * be used.
+             */
+
+            /* Verify that the listener type matches the destination type */
+            if (sel->u.listener->type != type) {
+                pj_lock_release(mgr->lock);
+                return PJSIP_ETPNOTSUITABLE;
+            }
+
+            /* We'll use this listener to create transport */
+            factory = sel->u.listener;
+
+            /* Verify if listener is still valid */
+            if (!pjsip_tpmgr_is_tpfactory_valid(mgr, factory)) {
+                pj_lock_release(mgr->lock);
+                PJ_LOG(3,(THIS_FILE, "Specified factory for creating "
+                                     "transport is not found"));
+                return PJ_ENOTFOUND;
+            }
+
+        } else {
+
+            /* Find factory with type matches the destination type */
+            factory = mgr->factory_list.next;
+            while (factory != &mgr->factory_list) {
+                if (factory->type == type)
+                    break;
+                factory = factory->next;
+            }
+
+            if (factory == &mgr->factory_list) {
+                /* No factory can create the transport! */
+                pj_lock_release(mgr->lock);
+                TRACE_((THIS_FILE, "No suitable factory was found either"));
+                return PJSIP_EUNSUPTRANSPORT;
+            }
         }
     }
@@ -2117 +2170 @@
             {pj_lock_release(mgr->lock); return PJ_EBUG;});
         pjsip_transport_add_ref(*tp);
+        (*tp)->factory = factory;
     }
     pj_lock_release(mgr->lock);