[sr-dev] [dialog MI dlg_end_dlg] error when terminating a dialog in state 2 (never removed from memory)

[Timo Reimann]

Hey Iñaki,


Iñaki Baz Castillo wrote:
> 2010/7/9 Iñaki Baz Castillo <ibc at aliax.net>:
>> However the current code could leak.
> 
> And IMHO it leaks!. The reason is that after the CANCEL the dialog
> information remains as follows:
> 
> dialog::  hash=3132:647756461
>         state:: 5
>         timestart:: 0
>         timeout:: 0
>         callid:: knfmgpcorrteiia at ibc-torre
>         from_uri:: sip:test_ibc at somedomain.org
>         from_tag:: vicxp
>         caller_contact:: sip:test_ibc at X.X.X.X
>         caller_cseq:: 326
>         caller_route_set::
>         caller_bind_addr:: udp:X.X.X.X:5060
>         to_uri:: sip:XXXXXX at somedomain.org
>         to_tag::
>         callee_contact::
>         callee_cseq::
>         callee_route_set::
>         callee_bind_addr::
> 
> This is, there is no timestart neither timeout values, so even if the
> expiration time for dialgo module is set to 60 seconds, the dialog
> remains in memory forever!

I don't think that it leaks. Please have a look at the
get_expired_dlgs(unsigned int time) function in dlg_timer.c: The loop
condition to get all expired dialogs is

  while( tl!=end && tl->timeout <= time) {
                    ^^^^^^^^^^^^^^^^^^^
    [...]
  }

The unit of "time" is ticks, so the while-loop picks those dialogs for
cleanup whose timeout value is lower or equal the current number of
ticks. Certainly, this is true for a timeout of zero.

I think it makes sense this way because dlg_end_dlg isn't supposed to
change the dialog state in case of failure (and checking
mi_terminate_dlg(), I believe it doesn't), so a subsequent,
UAC-initiated CANCEL shouldn't be any different from such a CANCEL not
involving a call to dlg_end_dlg(). A leak in this case would have been
detected already, hopefully.


Nevertheless, I think the current dialog module should be hotfixed such
that dlg_end_dlg doesn't end dialogs in the "early" state simply because
it's not capable of doing so. The upcoming, refurbished dialog module
should do better though.


Cheers,

--Timo