Browse code

dialog: keep slot locked when searching for duplicate dialog

- when attempting to create a new dialog, the function searching to see
if it is already one with same attributes keeps the slot locked so is
no race in between the return of function and building the new dlg
structure
- if the dlg is found, release the lock after figuring out it is a
spiral or not

Daniel-Constantin Mierla authored on 01/10/2015 13:49:26
Showing 3 changed files
... ...
@@ -766,7 +766,6 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs)
766 766
     str ttag;
767 767
     str req_uri;
768 768
     unsigned int dir;
769
-    int mlock;
770 769
 
771 770
 	dlg = dlg_get_ctx_dialog();
772 771
     if(dlg != NULL) {
... ...
@@ -792,14 +791,10 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs)
792 791
     trim(&req_uri);
793 792
 
794 793
 	dir = DLG_DIR_NONE;
795
-	mlock = 1;
796 794
 	/* search dialog by SIP attributes
797
-	 * - if not found, hash table slot is left locked, to avoid races
798
-	 *   to add 'same' dialog on parallel forking or not-handled-yet
799
-	 *   retransmissions. Release slot after linking new dialog */
800
-	dlg = search_dlg(&callid, &ftag, &ttag, &dir);
795
+	 * - hash table slot is left locked  */
796
+	dlg = dlg_search(&callid, &ftag, &ttag, &dir);
801 797
 	if(dlg) {
802
-		mlock = 0;
803 798
 		if (detect_spirals) {
804 799
 			if (spiral_detected == 1)
805 800
 				return 0;
... ...
@@ -817,13 +812,11 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs)
817 812
 				_dlg_ctx.iuid.h_id = dlg->h_id;
818 813
 				/* search_dlg() has incremented the ref count by 1 */
819 814
 				dlg_release(dlg);
815
+				dlg_hash_release(&callid);
820 816
 				return 0;
821 817
 			}
822 818
 			dlg_release(dlg);
823 819
 		}
824
-		/* lock the slot - dlg found, but in dlg_state_deleted, do a new one */
825
-		dlg_hash_lock(&callid);
826
-		mlock = 1;
827 820
     }
828 821
     spiral_detected = 0;
829 822
 
... ...
@@ -834,7 +827,7 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs)
834 827
                          &req_uri /*r-uri*/ );
835 828
 
836 829
 	if (dlg==0) {
837
-		if(likely(mlock==1)) dlg_hash_release(&callid);
830
+		dlg_hash_release(&callid);
838 831
 		LM_ERR("failed to create new dialog\n");
839 832
 		return -1;
840 833
 	}
... ...
@@ -842,7 +835,7 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs)
842 835
 	/* save caller's tag, cseq, contact and record route*/
843 836
 	if (populate_leg_info(dlg, req, t, DLG_CALLER_LEG,
844 837
 			&(get_from(req)->tag_value)) !=0) {
845
-		if(likely(mlock==1)) dlg_hash_release(&callid);
838
+		dlg_hash_release(&callid);
846 839
 		LM_ERR("could not add further info to the dialog\n");
847 840
 		shm_free(dlg);
848 841
 		return -1;
... ...
@@ -851,9 +844,10 @@ int dlg_new_dialog(sip_msg_t *req, struct cell *t, const int run_initial_cbs)
851 844
 	/* Populate initial varlist: */
852 845
 	dlg->vars = get_local_varlist_pointer(req, 1);
853 846
 
854
-	/* if search_dlg() returned NULL, slot was kept locked */
855
-	link_dlg(dlg, 0, mlock);
856
-	if(likely(mlock==1)) dlg_hash_release(&callid);
847
+	/* after dlg_search() slot was kept locked */
848
+	link_dlg(dlg, 0, 0);
849
+	/* unlock after dlg_search() */
850
+	dlg_hash_release(&callid);
857 851
 
858 852
 	dlg->lifetime = get_dlg_timeout(req);
859 853
 	s.s   = _dlg_ctx.to_route_name;
... ...
@@ -738,7 +738,7 @@ static inline struct dlg_cell* internal_get_dlg(unsigned int h_entry,
738 738
 		/* Check callid / fromtag / totag */
739 739
 		if (match_dialog( dlg, callid, ftag, ttag, dir)==1) {
740 740
 			ref_dlg_unsafe(dlg, 1);
741
-			dlg_unlock( d_table, d_entry);
741
+			if(likely(mode==0)) dlg_unlock( d_table, d_entry);
742 742
 			LM_DBG("dialog callid='%.*s' found on entry %u, dir=%d\n",
743 743
 				callid->len, callid->s,h_entry,*dir);
744 744
 			return dlg;
... ...
@@ -794,15 +794,15 @@ struct dlg_cell* get_dlg( str *callid, str *ftag, str *ttag, unsigned int *dir)
794 794
  * referred to as a dialog."
795 795
  * Note that the caller is responsible for decrementing (or reusing)
796 796
  * the reference counter by one again if a dialog has been found.
797
- * If the dialog is not found, the hash slot is left locked, to allow
798
- * linking the structure of a new dialog.
797
+ * Important: the hash slot is left locked (e.g., needed to allow
798
+ * linking the structure of a new dialog).
799 799
  * \param callid callid
800 800
  * \param ftag from tag
801 801
  * \param ttag to tag
802 802
  * \param dir direction
803 803
  * \return dialog structure on success, NULL on failure (and slot locked)
804 804
  */
805
-dlg_cell_t* search_dlg( str *callid, str *ftag, str *ttag, unsigned int *dir)
805
+dlg_cell_t* dlg_search( str *callid, str *ftag, str *ttag, unsigned int *dir)
806 806
 {
807 807
 	struct dlg_cell *dlg;
808 808
 	unsigned int he;
... ...
@@ -345,15 +345,15 @@ dlg_cell_t* get_dlg(str *callid, str *ftag, str *ttag, unsigned int *dir);
345 345
  * referred to as a dialog."
346 346
  * Note that the caller is responsible for decrementing (or reusing)
347 347
  * the reference counter by one again if a dialog has been found.
348
- * If the dialog is not found, the hash slot is left locked, to allow
349
- * linking the structure of a new dialog.
348
+ * Important: the hash slot is left locked (e.g., needed to allow
349
+ * linking the structure of a new dialog).
350 350
  * \param callid callid
351 351
  * \param ftag from tag
352 352
  * \param ttag to tag
353 353
  * \param dir direction
354 354
  * \return dialog structure on success, NULL on failure (and slot locked)
355 355
  */
356
-dlg_cell_t* search_dlg(str *callid, str *ftag, str *ttag, unsigned int *dir);
356
+dlg_cell_t* dlg_search(str *callid, str *ftag, str *ttag, unsigned int *dir);
357 357
 
358 358
 
359 359
 /*!