[sr-dev] modules_k/dialog: no referencing to current dialog possible when using dlg_manage()

[Timo Reimann]

Hi,

with respect to this thread on SR-Users

http://lists.sip-router.org/pipermail/sr-users/2011-March/067696.html

asking why set_dlg_profile() doesn't work in failure_route, I figured
out that the problem is related to dlg_manage() usage. Conceptually,
tracking dialogs on-demand via that function and using the dialog flag
should work equally well. However, the dialog creation process differs
regarding the parameters passed to dlg_new_dialog() (located in
dlg_handlers.c and called by both approaches) whose signature is

  int dlg_new_dialog(struct sip_msg *msg, struct cell *t)

When using dlg_manage(), a NULL pointer is passed instead of an existing
transaction cell. The pointer is subsequently passed to two tm module
callback registrations where one for the callback type TMCB_MAX is meant
as a "temporary workaround of missing dialog_ctx field" as noted by
Daniel in commit f6a66b42. However, that very registration call is bound
to a check that the transaction exists, i.e., parameter t is not NULL:

http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_k/dialog/dlg_handlers.c;h=a118a91e1e9e596425613e5be6ec53ba6483e9c1;hb=HEAD#l719

Consequently, no callback of type TMCB_MAX will be carried out for
dialogs generated from dlg_manage(). This poses to be a problem for a
bunch of dialog profiling-related exported functions as well as
is_known_dlg() since they rely on the get_current_dialog() function. For
non-REQUEST routes, that function accesses the current dialog by means
of iterating through all of the current transaction's callbacks looking
for one of type TMCB_MAX that was previously registered in
dlg_create_dialog(). However, as that registration never occurs in case
of dlg_manage()-based tracking for reasons lined out above, the set of
mentioned, exported functions cannot work. Refer to

http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=blob;f=modules_k/dialog/dlg_profile.c;h=a5f755882308be9787e871a254b87ac4372680e6;hb=HEAD#l355

to see how get_current_dialog() does it.

So how do we fix this?

My idea is to simply remove the not-NULL-check against the transaction t
prior to callback registration. If the transaction doesn't exists, the
tm module makes sure that it will be created lazily. This is how the
first callback registration for types TMCB_RESPONSE_READY and
TMCB_RESPONSE_FWDED in dlg_create_dialog() already works, apparently
with no issues. I also did a quick, simple test with the check left out
that proved to work fine.

As this looks like a fix to a work-around, however, I'd like to ask for
some feedback first.

Thanks and


cheers,

--Timo