Skip to content

Commit

Permalink
Fixed deadlock in CANCEL handling.
Browse files Browse the repository at this point in the history
Reported by Maxim Sobolev
  • Loading branch information
bogdan-iancu committed Aug 21, 2017
1 parent 7a5e2c7 commit 4aa5fb9
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 12 deletions.
2 changes: 1 addition & 1 deletion modules/tm/t_funcs.c
Expand Up @@ -231,7 +231,7 @@ int t_relay_to( struct sip_msg *p_msg , struct proxy_l *proxy, int flags)
if (flags&TM_T_REPLY_reason_FLAG) t->flags|=T_CANCEL_REASON_FLAG;

/* now go ahead and forward ... */
ret=t_forward_nonack( t, p_msg, proxy, 0/* no reset */);
ret=t_forward_nonack( t, p_msg, proxy, 0/*no reset*/, 0/*unlocked*/);
if (ret<=0) {
LM_DBG("t_forward_nonack returned error \n");
/* we don't want to pass upstream any reply regarding replicating
Expand Down
20 changes: 13 additions & 7 deletions modules/tm/t_fwd.c
Expand Up @@ -570,7 +570,7 @@ int t_add_reason(struct sip_msg *msg, char *val)


void cancel_invite(struct sip_msg *cancel_msg,
struct cell *t_cancel, struct cell *t_invite )
struct cell *t_cancel, struct cell *t_invite, int locked)
{
#define CANCEL_REASON_SIP_487 \
"Reason: SIP;cause=487;text=\"ORIGINATOR_CANCEL\"" CRLF
Expand All @@ -585,7 +585,10 @@ void cancel_invite(struct sip_msg *cancel_msg,
/* send back 200 OK as per RFC3261 */
reason.s = CANCELING;
reason.len = sizeof(CANCELING)-1;
t_reply( t_cancel, cancel_msg, 200, &reason );
if (locked)
t_reply_unsafe( t_cancel, cancel_msg, 200, &reason );
else
t_reply( t_cancel, cancel_msg, 200, &reason );

reason.s = NULL;
reason.len = 0;
Expand Down Expand Up @@ -648,7 +651,7 @@ void cancel_invite(struct sip_msg *cancel_msg,
* -1 - error during forward
*/
int t_forward_nonack( struct cell *t, struct sip_msg* p_msg ,
struct proxy_l * proxy, int reset_bcounter)
struct proxy_l * proxy, int reset_bcounter, int locked)
{
str reply_reason_487 = str_init("Request Terminated");
str backup_uri;
Expand Down Expand Up @@ -676,7 +679,7 @@ int t_forward_nonack( struct cell *t, struct sip_msg* p_msg ,
t_invite=t_lookupOriginalT( p_msg );
if (t_invite!=T_NULL_CELL) {
t_invite->flags |= T_WAS_CANCELLED_FLAG;
cancel_invite( p_msg, t, t_invite );
cancel_invite( p_msg, t, t_invite, locked );
return 1;
}
}
Expand All @@ -693,7 +696,10 @@ int t_forward_nonack( struct cell *t, struct sip_msg* p_msg ,
/* if no other signalling was performed on the transaction
* and the transaction was already canceled, better
* internally generate the 487 reply here */
t_reply( t, p_msg , 487 , &reply_reason_487);
if (locked)
t_reply_unsafe( t, p_msg , 487 , &reply_reason_487);
else
t_reply( t, p_msg , 487 , &reply_reason_487);
}
LM_INFO("discarding fwd for a cancelled transaction\n");
ser_error = E_NO_DESTINATION;
Expand Down Expand Up @@ -998,7 +1004,7 @@ int t_inject_branch( struct cell *t, struct sip_msg *msg, int flags)
}

/* generated the new branches, without branch counter reset */
rc = t_forward_nonack( t, &faked_req , NULL, 0 );
rc = t_forward_nonack( t, &faked_req , NULL, 0, 1/*locked*/ );

/* do we have to cancel the existing branches before injecting new ones? */
if (flags&TM_INJECT_FLAG_CANCEL) {
Expand Down Expand Up @@ -1061,7 +1067,7 @@ int t_replicate(struct sip_msg *p_msg, str *dst, int flags)

t->flags|=T_IS_LOCAL_FLAG;

return t_forward_nonack( t, p_msg, NULL, 1/*reset*/);
return t_forward_nonack( t, p_msg, NULL, 1/*reset*/, 0/*unlocked*/);
}
}

Expand Down
2 changes: 1 addition & 1 deletion modules/tm/t_fwd.h
Expand Up @@ -40,7 +40,7 @@ int add_blind_uac( );
int t_replicate(struct sip_msg *p_msg, str *dst, int flags);

int t_forward_nonack( struct cell *t, struct sip_msg* p_msg,
struct proxy_l * p, int reset_bcounter);
struct proxy_l * p, int reset_bcounter, int locked);

int add_phony_uac( struct cell *t);

Expand Down
2 changes: 1 addition & 1 deletion modules/tm/t_reply.c
Expand Up @@ -699,7 +699,7 @@ static inline int do_dns_failover(struct cell *t)
faked_req.force_send_socket = shmem_msg->force_send_socket;

/* send it out */
if (t_forward_nonack( t, &faked_req, uac->proxy, 1/*reset*/)==1)
if (t_forward_nonack( t, &faked_req, uac->proxy,1/*reset*/,1/*locked*/)==1)
ret = 0;

done:
Expand Down
4 changes: 2 additions & 2 deletions modules/tm/tm.c
Expand Up @@ -1313,13 +1313,13 @@ static int w_t_relay( struct sip_msg *p_msg , char *proxy, char *flags)
if (route_type==FAILURE_ROUTE) {
/* If called from failure route we need reset the branch counter to
* ignore the previous set of branches (already terminated) */
ret = t_forward_nonack( t, p_msg, p, 1/*reset*/);
ret = t_forward_nonack( t, p_msg, p, 1/*reset*/,1/*locked*/);
} else {
/* if called from request route and the transaction was previously
* created, better lock here to avoid any overlapping with
* branch injection from other processes */
LOCK_REPLIES(t);
ret = t_forward_nonack( t, p_msg, p, 1/*reset*/);
ret = t_forward_nonack( t, p_msg, p, 1/*reset*/,1/*locked*/);
UNLOCK_REPLIES(t);
}
if (ret<=0 ) {
Expand Down

0 comments on commit 4aa5fb9

Please sign in to comment.