Changeset 6037

Timestamp:

Jul 10, 2019 7:15:08 AM (5 years ago)

Author:

riza

Message:

Close #2211: Use group lock for SIP dialog.

Location:

pjproject/trunk/pjsip

Files:

• include/pjsip/sip_dialog.h (modified) (4 diffs)
• src/pjsip/sip_dialog.c (modified) (25 diffs)
• src/pjsua-lib/pjsua_media.c (modified) (2 diffs)

pjproject/trunk/pjsip/include/pjsip/sip_dialog.h

@@ -142 +142 @@
     char                obj_name[PJ_MAX_OBJ_NAME];  /**< Standard id.       */
     pj_pool_t          *pool;       /**< Dialog's pool.                     */
-    pj_mutex_t         *mutex_;     /**< Dialog's mutex. Do not call!!
-                                         Use pjsip_dlg_inc_lock() instead!  */
     pjsip_user_agent   *ua;         /**< User agent instance.               */
     pjsip_endpoint     *endpt;      /**< Endpoint instance.                 */
+    pj_grp_lock_t      *grp_lock_;  /**< Dialog's grp lock. Do not call!!
+                                         Use pjsip_dlg_inc_lock() instead!  */
 
     /** The dialog set which this dialog belongs (opaque type). */
@@ -192 +192 @@
     const void         *via_tp;     /**< Via transport.                     */
 };
+
+/**
+ * The parameter for \a pjsip_dlg_create_uac2().
+ */
+typedef struct pjsip_dlg_create_uac_param {
+    /**
+     * The user agent module instance.
+     */
+    pjsip_user_agent *ua;
+
+    /**
+     * Dialog local URI (i.e. From header).
+     */
+    pj_str_t local_uri;
+
+    /**
+     * Optional dialog local Contact to be put as Contact header value,
+     * hence the format must follow RFC 3261 Section 20.10:
+     * When the header field value contains a display name, the URI including
+     * all URI parameters is enclosed in "<" and ">".  If no "<" and ">" are
+     * present, all parameters after the URI are header parameters, not
+     * URI parameters.  The display name can be tokens, or a quoted string,
+     * if a larger character set is desired. If this argument is NULL,
+     * the Contact will be taken from the local URI.
+     */
+    pj_str_t local_contact;
+
+    /**
+     * Dialog remote URI (i.e. To header).
+     */
+    pj_str_t remote_uri;
+
+    /**
+     * Optional initial remote target. If this argument is NULL, the initial 
+     * target will be set to remote URI.
+     */
+    pj_str_t target;
+
+    /**
+     * Optional group lock to use by this dialog. If the value is NULL, 
+     * the dialog will create its own group lock.
+     */
+    pj_grp_lock_t *grp_lock;
+
+} pjsip_dlg_create_uac_param;
 
 
@@ -249 +294 @@
                                            pjsip_dialog **p_dlg);
 
+/**
+ * Variant of pjsip_dlg_create_uac() with additional parameter to specify
+ * the group lock to use. Group lock can be used to synchronize locking
+ * among several objects to prevent deadlock, and to synchronize the
+ * lifetime of objects sharing the same group lock.
+ *
+ * See \a pjsip_dlg_create_uac() for general info about this function.
+ *
+ * @param param             The parameter, refer to
+ *                          \a pjsip_dlg_create_uac_param
+ * @param p_dlg             Pointer to receive the dialog.
+ *
+ * @return                  PJ_SUCCESS on success.
+ */
+PJ_DECL(pj_status_t) pjsip_dlg_create_uac2(
+                                const pjsip_dlg_create_uac_param *create_param,
+                                pjsip_dialog **p_dlg);
+
 
 #if !DEPRECATED_FOR_TICKET_1902
@@ -539 +602 @@
 PJ_DECL(void) pjsip_dlg_dec_lock( pjsip_dialog *dlg );
 
+/**
+ * Get the group lock for the SIP dialog. Note that prior to calling this
+ * method, it is recommended to hold reference to the dialog
+ * (e.g: call #pjsip_dlg_inc_session() or #pjsip_dlg_inc_lock()).
+ *
+ * @param dlg               The dialog.
+ *
+ * @return                  The group lock.
+ */
+PJ_DECL(pj_grp_lock_t *) pjsip_dlg_get_lock( pjsip_dialog *dlg );
+
 
 /**

pjproject/trunk/pjsip/src/pjsip/sip_dialog.c

@@ -62 +62 @@
 }
 
+static void dlg_on_destroy( void *arg )
+{
+    pjsip_dialog *dlg = (pjsip_dialog *)arg;
+
+    PJ_LOG(5,(dlg->obj_name, "Dialog destroyed!"));
+
+    pjsip_endpt_release_pool(dlg->endpt, dlg->pool);
+}
+
 static pj_status_t create_dialog( pjsip_user_agent *ua,
+                                  pj_grp_lock_t *grp_lock,
                                   pjsip_dialog **p_dlg)
 {
@@ -99 +109 @@
         goto on_error;
 
-    status = pj_mutex_create_recursive(pool, dlg->obj_name, &dlg->mutex_);
-    if (status != PJ_SUCCESS)
-        goto on_error;
+    if (grp_lock) {
+        dlg->grp_lock_ = grp_lock;
+    } else {
+        status = pj_grp_lock_create(pool, NULL, &dlg->grp_lock_);
+        if (status != PJ_SUCCESS) {
+            goto on_error;
+        }
+    }
+
+    pj_grp_lock_add_ref(dlg->grp_lock_);
+    pj_grp_lock_add_handler(dlg->grp_lock_, pool, dlg, &dlg_on_destroy);
 
     pjsip_target_set_init(&dlg->target_set);
@@ -109 +127 @@
 
 on_error:
-    if (dlg->mutex_)
-        pj_mutex_destroy(dlg->mutex_);
     pjsip_endpt_release_pool(endpt, pool);
     return status;
@@ -117 +133 @@
 static void destroy_dialog( pjsip_dialog *dlg, pj_bool_t unlock_mutex )
 {
-    if (dlg->mutex_) {
-        if (unlock_mutex) pj_mutex_unlock(dlg->mutex_);
-        pj_mutex_destroy(dlg->mutex_);
-        dlg->mutex_ = NULL;
-    }
     if (dlg->tp_sel.type != PJSIP_TPSELECTOR_NONE) {
         pjsip_tpselector_dec_ref(&dlg->tp_sel);
@@ -127 +138 @@
     }
     pjsip_auth_clt_deinit(&dlg->auth_sess);
-    pjsip_endpt_release_pool(dlg->endpt, dlg->pool);
-}
-
+
+    pj_grp_lock_dec_ref(dlg->grp_lock_);
+
+    if (unlock_mutex)
+        pj_grp_lock_release(dlg->grp_lock_);
+}
 
 /*
@@ -141 +155 @@
                                           pjsip_dialog **p_dlg)
 {
+    PJ_ASSERT_RETURN(ua && local_uri && remote_uri && p_dlg, PJ_EINVAL);
+
+    pjsip_dlg_create_uac_param create_param;
+    pj_bzero(&create_param, sizeof(create_param));
+
+    create_param.ua = ua;
+    create_param.local_uri = *local_uri;
+    create_param.remote_uri = *remote_uri;
+    if (local_contact)
+        create_param.local_contact = *local_contact;
+
+    if (target)
+        create_param.target = *target;
+
+    return pjsip_dlg_create_uac2(&create_param, p_dlg);
+}
+
+PJ_DEF(pj_status_t) pjsip_dlg_create_uac2(
+                                const pjsip_dlg_create_uac_param *create_param,
+                                pjsip_dialog **p_dlg)
+{
     pj_status_t status;
     pj_str_t tmp;
@@ -146 +181 @@
 
     /* Check arguments. */
-    PJ_ASSERT_RETURN(ua && local_uri && remote_uri && p_dlg, PJ_EINVAL);
+    PJ_ASSERT_RETURN(create_param->ua && create_param->local_uri.slen &&
+                     create_param->remote_uri.slen && p_dlg, PJ_EINVAL);
 
     /* Create dialog instance. */
-    status = create_dialog(ua, &dlg);
+    status = create_dialog(create_param->ua, create_param->grp_lock, &dlg);
     if (status != PJ_SUCCESS)
         return status;
 
     /* Parse target. */
-    pj_strdup_with_null(dlg->pool, &tmp, target ? target : remote_uri);
+    pj_strdup_with_null(dlg->pool, &tmp, create_param->target.slen ?
+                        &create_param->target : &create_param->remote_uri);
     dlg->target = pjsip_parse_uri(dlg->pool, tmp.ptr, tmp.slen, 0);
     if (!dlg->target) {
@@ -204 +241 @@
     /* Init local info. */
     dlg->local.info = pjsip_from_hdr_create(dlg->pool);
-    pj_strdup_with_null(dlg->pool, &dlg->local.info_str, local_uri);
+    pj_strdup_with_null(dlg->pool, &dlg->local.info_str,
+                        &create_param->local_uri);
     dlg->local.info->uri = pjsip_parse_uri(dlg->pool,
                                            dlg->local.info_str.ptr,
@@ -226 +264 @@
     /* Init local contact. */
     pj_strdup_with_null(dlg->pool, &tmp,
-                        local_contact ? local_contact : local_uri);
+                    create_param->local_contact.slen ?
+                    &create_param->local_contact : &create_param->local_uri);
     dlg->local.contact = (pjsip_contact_hdr*)
                          pjsip_parse_hdr(dlg->pool, &HCONTACT, tmp.ptr,
@@ -237 +276 @@
     /* Init remote info. */
     dlg->remote.info = pjsip_to_hdr_create(dlg->pool);
-    pj_strdup_with_null(dlg->pool, &dlg->remote.info_str, remote_uri);
+    pj_strdup_with_null(dlg->pool, &dlg->remote.info_str,
+                        &create_param->remote_uri);
     dlg->remote.info->uri = pjsip_parse_uri(dlg->pool,
                                             dlg->remote.info_str.ptr,
@@ -293 +333 @@
 
     /* Register this dialog to user agent. */
-    status = pjsip_ua_register_dlg( ua, dlg );
+    status = pjsip_ua_register_dlg( create_param->ua, dlg );
     if (status != PJ_SUCCESS)
         goto on_error;
 
-
     /* Done! */
     *p_dlg = dlg;
-
 
     PJ_LOG(5,(dlg->obj_name, "UAC dialog created"));
@@ -350 +388 @@
 
     /* Create dialog instance. */
-    status = create_dialog(ua, &dlg);
+    status = create_dialog(ua, NULL, &dlg);
     if (status != PJ_SUCCESS)
         return status;
@@ -565 +603 @@
         destroy_dialog(dlg, PJ_FALSE);
     }
-    
+
     return status;
 }
@@ -681 +719 @@
 
     /* Create the dialog. */
-    status = create_dialog((pjsip_user_agent*)first_dlg->ua, &dlg);
+    status = create_dialog((pjsip_user_agent*)first_dlg->ua, NULL, &dlg);
     if (status != PJ_SUCCESS)
         return status;
@@ -772 +810 @@
 
 /*
- * Destroy dialog.
+ * Unregister and destroy dialog.
  */
 static pj_status_t unregister_and_destroy_dialog( pjsip_dialog *dlg,
@@ -797 +835 @@
     }
 
-    /* Log */
-    PJ_LOG(5,(dlg->obj_name, "Dialog destroyed"));
-
     /* Destroy this dialog. */
     destroy_dialog(dlg, unlock_mutex);
@@ -879 +914 @@
 
 /*
- * Lock dialog and increment session counter temporarily
+ * Lock dialog and increment session count temporarily
  * to prevent it from being deleted. In addition, it must lock
  * the user agent's dialog table first, to prevent deadlock.
@@ -888 +923 @@
               dlg->sess_count));
 
-    pj_mutex_lock(dlg->mutex_);
+    pj_grp_lock_acquire(dlg->grp_lock_);
     dlg->sess_count++;
 
@@ -895 +930 @@
 }
 
-/* Try to acquire dialog's mutex, but bail out if mutex can not be
+/* Try to acquire dialog's group lock, but bail out if group lock can not be
  * acquired immediately.
  */
@@ -905 +940 @@
               dlg->sess_count));
 
-    status = pj_mutex_trylock(dlg->mutex_);
+    status = pj_grp_lock_tryacquire(dlg->grp_lock_);
     if (status != PJ_SUCCESS) {
         PJ_LOG(6,(dlg->obj_name, "pjsip_dlg_try_inc_lock() failed"));
@@ -921 +956 @@
 
 /*
- * Unlock dialog and decrement session counter.
+ * Unlock dialog and decrement reference counter.
  * It may delete the dialog!
  */
@@ -935 +970 @@
 
     if (dlg->sess_count==0 && dlg->tsx_count==0) {
-        pj_mutex_unlock(dlg->mutex_);
-        pj_mutex_lock(dlg->mutex_);
-        /* We are holding the dialog mutex here, so before we destroy
+        pj_grp_lock_release(dlg->grp_lock_);
+        pj_grp_lock_acquire(dlg->grp_lock_);
+        /* We are holding the dialog group lock here, so before we destroy
          * the dialog, make sure that we unlock it first to avoid
          * undefined behaviour on some platforms. See ticket #1886.
@@ -943 +978 @@
         unregister_and_destroy_dialog(dlg, PJ_TRUE);
     } else {
-        pj_mutex_unlock(dlg->mutex_);
+        pj_grp_lock_release(dlg->grp_lock_);
     }
 
@@ -950 +985 @@
 
 
-
-/*
- * Decrement session counter.
+/*
+ * Decrement session count.
  */
 PJ_DEF(pj_status_t) pjsip_dlg_dec_session( pjsip_dialog *dlg,
@@ -970 +1004 @@
     pj_log_pop_indent();
     return PJ_SUCCESS;
+}
+
+PJ_DEF(pj_grp_lock_t *) pjsip_dlg_get_lock(pjsip_dialog *dlg)
+{
+    PJ_ASSERT_RETURN(dlg, NULL);
+    return dlg->grp_lock_;
 }
 

pjproject/trunk/pjsip/src/pjsua-lib/pjsua_media.c

@@ -1119 +1119 @@
              * https://trac.pjsip.org/repos/ticket/1737
              */
-            ++dlg->sess_count;
+            pjsip_dlg_inc_session(dlg, &pjsua_var.mod);
             pjsip_dlg_dec_lock(dlg);
         }
@@ -1127 +1127 @@
         if (dlg) {
             pjsip_dlg_inc_lock(dlg);
-            --dlg->sess_count;
+            pjsip_dlg_dec_session(dlg, &pjsua_var.mod);
         }
         if (has_pjsua_lock)