Hi Sven,

sorry, took a while to get back to this subject.

The fix seems ok, being tested a bit in my side. I didn't wanted to backport it immediately, just to give a second thought about it. The purpose was to backport, of course.

Btw, have you had a chance to test it?

If you need it in 3.2 quickly, go ahead and cherry-pick it on stable branch. If not, I will do it, probably soon as well.

Cheers,
Daniel

On 11/2/11 4:27 PM, Sven Knoblich wrote:

Hi Daniel,
i saw your changes regarding the stateless reply in dialog-module. Should we push this commit as well into the 3.2 branch?

Thanks for your reply.

Bye,
Sven



Am 27.10.2011 16:16, schrieb Daniel-Constantin Mierla:

Module: sip-router
Branch: master
Commit: ad4cfe8a3410059bc5c0b4951e49a952c4b01dfe
URL:    http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=ad4cfe8a3410059bc5c0b4951e49a952c4b01dfe

Author: Daniel-Constantin Mierla <miconda@gmail.com>
Committer: Daniel-Constantin Mierla <miconda@gmail.com>
Date:   Thu Oct 27 07:17:52 2011 +0200

dialog(k): postpone setting tm callbacks until T is created

- add dialog in tm callbacks when transaction is created
- when using dlg_manage(), if dialog does not makes it to transaction,
  clean it up
- fixes case when stateless reply is used after dlg_manage() to create a
  new dialog

---

 modules_k/dialog/dialog.c       |    3 +-
 modules_k/dialog/dlg_handlers.c |   95 ++++++++++++++++++++++++++-------------
 modules_k/dialog/dlg_hash.h     |    2 +
 modules_k/dialog/dlg_profile.c  |    7 +++-
 4 files changed, 73 insertions(+), 34 deletions(-)

diff --git a/modules_k/dialog/dialog.c b/modules_k/dialog/dialog.c
index 7d8f239..14e8d81 100644
--- a/modules_k/dialog/dialog.c
+++ b/modules_k/dialog/dialog.c
@@ -507,7 +507,8 @@ static int mod_init(void)
 	}
 
 	if (initial_cbs_inscript != 0 && initial_cbs_inscript != 1) {
-		LM_ERR("invalid parameter for running initial callbacks in-script (must be either 0 or 1)\n");
+		LM_ERR("invalid parameter for running initial callbacks in-script"
+				" (must be either 0 or 1)\n");
 		return -1;
 	}
 
diff --git a/modules_k/dialog/dlg_handlers.c b/modules_k/dialog/dlg_handlers.c
index ccc95f1..701d80c 100644
--- a/modules_k/dialog/dlg_handlers.c
+++ b/modules_k/dialog/dlg_handlers.c
@@ -106,6 +106,8 @@ static unsigned int CURR_DLG_ID  = 0xffffffff;	/*!< current dialog id */
 /*! separator inside the record-route paramter */
 #define DLG_SEPARATOR      '.'
 
+int dlg_set_tm_callbacks(tm_cell_t *t, sip_msg_t *req, dlg_cell_t *dlg,
+		int mode);
 
 /*!
  * \brief Initialize the dialog handlers
@@ -609,19 +611,28 @@ static inline int pre_match_parse( struct sip_msg *req, str *callid,
  */
 void dlg_onreq(struct cell* t, int type, struct tmcb_params *param)
 {
-	struct sip_msg *req = param->req;
+	sip_msg_t *req = param->req;
 
-	if (!initial_cbs_inscript) {
-		if (spiral_detected == 1)
-			run_dlg_callbacks( DLGCB_SPIRALED, current_dlg_pointer, req, NULL, DLG_DIR_DOWNSTREAM, 0);
-		else if (spiral_detected == 0)
-			run_create_callbacks( current_dlg_pointer, req);
-	}
-	if((req->flags&dlg_flag)!=dlg_flag)
-		return;
-	if (current_dlg_pointer!=NULL)
+	if(req->first_line.u.request.method_value != METHOD_INVITE)
 		return;
-	dlg_new_dialog(req, t, 1);
+
+	if (current_dlg_pointer!=NULL) {
+		if (!initial_cbs_inscript) {
+			if (spiral_detected == 1)
+				run_dlg_callbacks( DLGCB_SPIRALED, current_dlg_pointer,
+						req, NULL, DLG_DIR_DOWNSTREAM, 0);
+			else if (spiral_detected == 0)
+				run_create_callbacks( current_dlg_pointer, req);
+		}
+	}
+	if (current_dlg_pointer==NULL) {
+		if((req->flags&dlg_flag)!=dlg_flag)
+			return;
+		dlg_new_dialog(req, t, 1);
+	}
+	if (current_dlg_pointer!=NULL) {
+		dlg_set_tm_callbacks(t, req, current_dlg_pointer, spiral_detected);
+	}
 }
 
 
@@ -782,9 +793,10 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb
             spiral_detected = 1;
 
             if (run_initial_cbs)
-                run_dlg_callbacks( DLGCB_SPIRALED, dlg, req, NULL, DLG_DIR_DOWNSTREAM, 0);
-            // get_dlg has incremented the ref count by 1
-            unref_dlg(dlg, 1);
+                run_dlg_callbacks( DLGCB_SPIRALED, dlg, req, NULL,
+						DLG_DIR_DOWNSTREAM, 0);
+            /* get_dlg() has incremented the ref count by 1
+			 * - it's ok, dlg will be used to set current_dialog_pointer */
             goto finish;
         }
     }
@@ -811,7 +823,6 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb
 		return -1;
 	}
 
-
 	/* Populate initial varlist: */
 	dlg->vars = get_local_varlist_pointer(req, 1);
 
@@ -826,13 +837,7 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb
 		goto error;
 	}
 
-	if ( d_tmb.register_tmcb( req, t,
-				TMCB_RESPONSE_READY|TMCB_RESPONSE_FWDED,
-				dlg_onreply, (void*)dlg, unref_new_dialog)<0 ) {
-		LM_ERR("failed to register TMCB\n");
-		goto error;
-	}
-    // increase reference counter because of registered callback
+	/* reference it once for current_dialog_pointer */
     ref_dlg(dlg, 1);
 
 	dlg->lifetime = get_dlg_timeout(req);
@@ -847,6 +852,40 @@ int dlg_new_dialog(struct sip_msg *req, struct cell *t, const int run_initial_cb
     if_update_stat( dlg_enable_stats, processed_dlgs, 1);
 
 finish:
+    set_current_dialog(req, dlg);
+    _dlg_ctx.dlg = dlg;
+
+	return 0;
+
+error:
+	if (!spiral_detected)
+		unref_dlg(dlg,1);               // undo ref regarding linking
+	return -1;
+}
+
+
+/*!
+ * \brief add dlg structure to tm callbacks
+ * \param t current transaction
+ * \param req current sip request
+ * \param dlg current dialog
+ * \param smode if the sip request was spiraled
+ * \return 0 on success, -1 on failure
+ */
+int dlg_set_tm_callbacks(tm_cell_t *t, sip_msg_t *req, dlg_cell_t *dlg,
+		int smode)
+{
+	if(smode==0) {
+		if ( d_tmb.register_tmcb( req, t,
+				TMCB_RESPONSE_READY|TMCB_RESPONSE_FWDED,
+				dlg_onreply, (void*)dlg, unref_new_dialog)<0 ) {
+			LM_ERR("failed to register TMCB\n");
+			goto error;
+		}
+		// increase reference counter because of registered callback
+		ref_dlg(dlg, 1);
+	}
+
 	if (t) {
 		// transaction exists ==> keep ref counter large enough to
 		// avoid premature cleanup and ensure proper dialog referencing
@@ -854,9 +893,7 @@ finish:
 			LM_ERR("failed to store dialog in transaction\n");
 			goto error;
 		}
-	}
-	else
-	{
+	} else {
 		// no transaction exists ==> postpone work until we see the
 		// request being forwarded statefully
         if ( d_tmb.register_tmcb( req, NULL, TMCB_REQUEST_FWDED,
@@ -865,16 +902,10 @@ finish:
 			goto error;
         }
 	}
-
-    set_current_dialog(req, dlg);
-    _dlg_ctx.dlg = dlg;
-    ref_dlg(dlg, 1);
+	dlg->dflags |= DLG_FLAG_TM;
 
 	return 0;
-
 error:
-	if (!spiral_detected)
-		unref_dlg(dlg,1);               // undo ref regarding linking
 	return -1;
 }
 
diff --git a/modules_k/dialog/dlg_hash.h b/modules_k/dialog/dlg_hash.h
index ecb6526..9a8854c 100644
--- a/modules_k/dialog/dlg_hash.h
+++ b/modules_k/dialog/dlg_hash.h
@@ -79,6 +79,8 @@
 /* dialog-variable flags (in addition to dialog-flags) */
 #define DLG_FLAG_DEL           (1<<8) /*!< delete this var */
 
+#define DLG_FLAG_TM            (1<<9) /*!< dialog is set in transaction */
+
 #define DLG_CALLER_LEG         0 /*!< attribute that belongs to a caller leg */
 #define DLG_CALLEE_LEG         1 /*!< attribute that belongs to a callee leg */
 
diff --git a/modules_k/dialog/dlg_profile.c b/modules_k/dialog/dlg_profile.c
index 2e69291..bb20c57 100644
--- a/modules_k/dialog/dlg_profile.c
+++ b/modules_k/dialog/dlg_profile.c
@@ -312,7 +312,12 @@ int profile_cleanup( struct sip_msg *msg, unsigned int flags, void *param )
 {
 	current_dlg_msg_id = 0;
 	if (current_dlg_pointer) {
-		unref_dlg( current_dlg_pointer, 1);
+		if(current_dlg_pointer->dflags & DLG_FLAG_TM) {
+			unref_dlg( current_dlg_pointer, 1);
+		} else {
+			/* dialog didn't make it to tm */
+			unref_dlg( current_dlg_pointer, 2);
+		}
 		current_dlg_pointer = NULL;
 	}
 	if (current_pending_linkers) {


_______________________________________________
sr-dev mailing list
sr-dev@lists.sip-router.org
http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla -- http://www.asipto.com
Kamailio Advanced Training, Dec 5-8, Berlin: http://asipto.com/u/kat
http://linkedin.com/in/miconda -- http://twitter.com/miconda