mirror of
https://github.com/asterisk/asterisk.git
synced 2025-10-12 15:45:18 +00:00
Merged revisions 351183 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/10 ................ r351183 | russell | 2012-01-16 20:43:19 -0500 (Mon, 16 Jan 2012) | 29 lines Merged revisions 351182 via svnmerge from https://origsvn.digium.com/svn/asterisk/branches/1.8 ........ r351182 | russell | 2012-01-16 20:37:03 -0500 (Mon, 16 Jan 2012) | 22 lines Add some missing locking in chan_sip. This patch adds some missing locking to the function send_provisional_keepalive_full(). This function is called from the scheduler, which is processed in the SIP monitor thread. The associated channel (or pbx) thread will also be using the same sip_pvt and ast_channel so locking must be used. The sip_pvt_lock_full() function is used to ensure proper locking order in a safe manner. In passing, document a suspected reference counting error in this function. The "fix" is left commented out because when the "fix" is present, crashes occur. My theory is that fixing it is exposing a reference counting error elsewhere, but I don't know where. (Or my analysis of this being a problem could have been completely wrong in the first place). Leave the comment in the code for so that someone may investigate it again in the future. Also add a bit of doxygen to transmit_provisional_response(). (closes issue ASTERISK-18979) Review: https://reviewboard.asterisk.org/r/1648 ........ ................ git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@351184 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
@@ -4099,6 +4099,10 @@ static void add_blank(struct sip_request *req)
|
||||
static int send_provisional_keepalive_full(struct sip_pvt *pvt, int with_sdp)
|
||||
{
|
||||
const char *msg = NULL;
|
||||
struct ast_channel *chan;
|
||||
int res = 0;
|
||||
|
||||
chan = sip_pvt_lock_full(pvt);
|
||||
|
||||
if (!pvt->last_provisional || !strncasecmp(pvt->last_provisional, "100", 3)) {
|
||||
msg = "183 Session Progress";
|
||||
@@ -4110,10 +4114,35 @@ static int send_provisional_keepalive_full(struct sip_pvt *pvt, int with_sdp)
|
||||
} else {
|
||||
transmit_response(pvt, S_OR(msg, pvt->last_provisional), &pvt->initreq);
|
||||
}
|
||||
return PROVIS_KEEPALIVE_TIMEOUT;
|
||||
res = PROVIS_KEEPALIVE_TIMEOUT;
|
||||
}
|
||||
|
||||
return 0;
|
||||
if (chan) {
|
||||
ast_channel_unlock(chan);
|
||||
chan = ast_channel_unref(chan);
|
||||
}
|
||||
|
||||
if (!res) {
|
||||
pvt->provisional_keepalive_sched_id = -1;
|
||||
}
|
||||
|
||||
sip_pvt_unlock(pvt);
|
||||
|
||||
#if 0
|
||||
/*
|
||||
* XXX BUG TODO
|
||||
*
|
||||
* Without this code, it appears as if this function is leaking its
|
||||
* reference to the sip_pvt. However, adding it introduces a crash.
|
||||
* This points to some sort of reference count imbalance elsewhere,
|
||||
* but I'm not sure where ...
|
||||
*/
|
||||
if (!res) {
|
||||
dialog_unref(pvt, "dialog ref for provisional keepalive");
|
||||
}
|
||||
#endif
|
||||
|
||||
return res;
|
||||
}
|
||||
|
||||
static int send_provisional_keepalive(const void *data)
|
||||
@@ -10916,7 +10945,13 @@ static void get_realm(struct sip_pvt *p, const struct sip_request *req)
|
||||
ast_string_field_set(p, realm, sip_cfg.realm);
|
||||
}
|
||||
|
||||
/* Only use a static string for the msg, here! */
|
||||
/*!
|
||||
* \internal
|
||||
*
|
||||
* \arg msg Only use a string constant for the msg, here, it is shallow copied
|
||||
*
|
||||
* \note assumes the sip_pvt is locked.
|
||||
*/
|
||||
static int transmit_provisional_response(struct sip_pvt *p, const char *msg, const struct sip_request *req, int with_sdp)
|
||||
{
|
||||
int res;
|
||||
|
Reference in New Issue
Block a user