sysmocom
/
asterisk
11
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Update some comments and resolve potential memory corruption in chan_sip. 

While browsing chan_sip the other day, I noticed this dangerous code in
dialog_needdestroy().  This function is an ao2_callback.  It is absolutely
_not_ okay to unlock the container from within this function.  It's also not
clear why it was useful.  Given that it could cause memory corruption, I have
removed it.

There was also a TODO comment left describing a potential implementation of
an improvement to the needdestroy handling.  I'm not convinced that what was
described is the best choice here, so I have briefly described the way that
this function is used today that could be improved.


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@186928 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Russell Bryant 2009-04-08 12:35:57 +00:00
parent b289374dfe
commit 0fab071d13
1 changed files with 13 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
channels/chan_sip.c
View File

@ -14341,41 +14341,26 @@ static void cleanup_stale_contexts(char *new, char *old)
}
}
/*! \brief this func is used with ao2_callback to unlink/delete all dialogs that
are marked needdestroy. It will return CMP_MATCH for candidates, and they
will be unlinked
TODO: Implement a queue and instead of marking a dialog
to be destroyed, toss it into the queue. Have a separate
thread do the locking and destruction */
/*!
* \brief Match dialogs that need to be destroyed
*
* \details This is used with ao2_callback to unlink/delete all dialogs that
* are marked needdestroy. It will return CMP_MATCH for candidates, and they
* will be unlinked.
*
* \todo Re-work this to improve efficiency. Currently, this function is called
* on _every_ dialog after processing _every_ incoming SIP/UDP packet, or
* potentially even more often when the scheduler has entries to run.
*/
static int dialog_needdestroy(void *dialogobj, void *arg, int flags)
{
struct sip_pvt *dialog = dialogobj;
time_t *t = arg;
/* log_show_lock(ao2_object_get_lockaddr(dialog)); this is an example of how to use log_show_lock() */
if (sip_pvt_trylock(dialog)) {
/* In very short-duration calls via sipp,
this path gets executed fairly frequently (3% or so) even in low load
situations; the routines we most commonly fight for a lock with:
sip_answer (7 out of 9)
sip_hangup (2 out of 9)
*/
ao2_unlock(dialogs);
usleep(1);
ao2_lock(dialogs);
/* I had previously returned CMP_STOP here; but changed it to return
a zero instead, because there is no need to stop at the first sign
of trouble. The old algorithm would restart in such circumstances,
but there is no need for this now in astobj2-land. We'll
just skip over this element this time, and catch it on the
next traversal, which is about once a second or so. See the
ao2_callback call in do_monitor. We don't want the number of
dialogs to back up too much between runs.
*/
/* Don't block the monitor thread. This function is called often enough
* that we can wait for the next time around. */
return 0;
}