[Asterisk-code-review] res pjsip rfc3326.c: Fix crash when channel goes away. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: res_pjsip_rfc3326.c: Fix crash when channel goes away.
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/1626/1/res/res_pjsip_rfc3326.c
File res/res_pjsip_rfc3326.c:

Line 105: 	if ((pjsip_method_cmp(&tdata->msg->line.req.method, &pjsip_bye_method)
        : 			&& pjsip_method_cmp(&tdata->msg->line.req.method, &pjsip_cancel_method))
        : 		|| !session->channel
        : 		/*
        : 		 * The session->channel has been seen to go away on us between
        : 		 * checks so we must also be running under the call's serializer
        : 		 * thread.
        : 		 */
        : 		|| session->serializer != ast_threadpool_serializer_get_current()) {
        : 		return;
        : 	}
> If the channel can go away on us between checks, should something be holdin
The problem is that the crash happens under exceptional circumstances.  PJSIP/PJPROJECT sets up and executes the transaction timeout.  The transaction timeout code is run by the monitor thread which hangs up the call and generates an outgoing BYE message.  The monitor thread and the call's serializer thread are then in a race.  The monitor thread executes the supplement to add the Reason header with the cause code set on the channel and the call's searializer thread is tearing down the channel.

The potential is there for any outgoing supplement that needs to access the session's channel when sending a message triggered by PJSIP/PJPROJECT.  However, it is only expected to happen with a supplement for a BYE or CANCEL message when the channel would be going away.  Fortunately, the other outgoing supplements that do anything with the channel only deal with REFER, INVITE, and UPDATE messages.  I'm not aware of any reason that PJSIP/PJPROJECT would send one of those messages itself so the executing thread would be the call's searializer.


-- 
To view, visit https://gerrit.asterisk.org/1626
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I866388d2b97ea2032eaae3f3ab3f1ca6cbd2df89
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Richard Mudgett <rmudgett at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes