[Asterisk-code-review] chan iax2: Prevent deadlock between hangup and sending lagrq... (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change.

Change subject: chan_iax2: Prevent deadlock between hangup and sending lagrq/ping
......................................................................


Patch Set 13: Code-Review-1

(4 comments)

https://gerrit.asterisk.org/#/c/169/13/channels/chan_iax2.c
File channels/chan_iax2.c:

Line 2073: 	int sched_id = (int)((long)(data));
Unnecessary parentheses:

int sched_id = (int)(long) data;


Line 2075: 	/* If call is still found in scheduler (not executed yet), delete it. */
         : 	ast_sched_del_if_exist(sched, sched_id);
Deleting the assert in _ast_sched_del() allows this to just be:

AST_SCHED_DEL();
return 0;


Line 2356: 	iax2_delete_from_sched((void*)(long)iaxs[callno]->lagid);
         : 	iax2_delete_from_sched((void*)(long)iaxs[callno]->pingid);
         : 	iaxs[callno]->lagid = -1;
         : 	iaxs[callno]->pingid = -1;
Removing the assert associated with AST_SCHED_DEL() means this doesn't need to be changed anymore.


https://gerrit.asterisk.org/#/c/169/13/main/sched.c
File main/sched.c:

Line 531: #ifndef AST_DEVMODE
        : 		ast_assert(s != NULL);
        : #else
        : 		{
        : 			char buf[100];
        : 
        : 			snprintf(buf, sizeof(buf), "s != NULL, id=%d", id);
        : 			_ast_assert(0, buf, file, line, function);
        : 		}
        : #endif
I think it would be better to just delete this assert instead of adding the new API call.  This assert seems to be the reason for a lot of awkward code to avoid it.  Then iax2_delete_from_sched() in chan_iax2.c can just be:

AST_SCHED_DEL()
return 0;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I03bec1fc8faacb89630269e935fa667c6d6c080c
Gerrit-PatchSet: 13
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Y Ateya <y.ateya at starkbits.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: George Joseph <george.joseph at fairview5.com>
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-Reviewer: Y Ateya <y.ateya at starkbits.com>
Gerrit-HasComments: Yes