Index: channels/chan_iax2.c =================================================================== diff -u -N -r55c8daf88b94f74e334ab246c51bdb7b469eedc4 -rcf98c744d5bb7e5f015f201bc79c58462f7aaaed --- a/channels/chan_iax2.c (.../chan_iax2.c) (revision 55c8daf88b94f74e334ab246c51bdb7b469eedc4) +++ b/channels/chan_iax2.c (.../chan_iax2.c) (revision cf98c744d5bb7e5f015f201bc79c58462f7aaaed) @@ -395,8 +395,6 @@ static struct io_context *io; static struct ast_sched_context *sched; -#define DONT_RESCHEDULE -2 - static iax2_format iax2_capability = IAX_CAPABILITY_FULLBANDWIDTH; static int iaxdebug = 0; @@ -864,6 +862,8 @@ int frames_dropped; /*! received frame count: (just for stats) */ int frames_received; + /*! Destroying this call initiated. */ + int destroy_initiated; /*! num bytes used for calltoken ie, even an empty ie should contain 2 */ unsigned char calltoken_ie_len; /*! hold all signaling frames from the pbx thread until we have a destination callno */ @@ -1672,37 +1672,55 @@ return ast_sched_add(con, when, callback, data); } +/* + * \brief Acquire the iaxsl[callno] if call exists and not having ongoing hangup. + * \param callno Call number to lock. + * \return 0 If call disappeared or has ongoing hangup procedure. 1 If call found and mutex is locked. + */ +static int iax2_lock_callno_unless_destroyed(int callno) +{ + ast_mutex_lock(&iaxsl[callno]); + + /* We acquired the lock; but the call was already destroyed (we came after full hang up procedures) + * or destroy initiated (in middle of hang up procedure. */ + if (!iaxs[callno] || iaxs[callno]->destroy_initiated) { + ast_debug(3, "I wanted to lock callno %d, but it is dead or going to die.\n", callno); + ast_mutex_unlock(&iaxsl[callno]); + return 0; + } + + /* Lock acquired, and callno is alive and kicking. */ + return 1; +} + static int send_ping(const void *data); static void __send_ping(const void *data) { - int callno = (long) data; + int callno = PTR_TO_CALLNO(data); - ast_mutex_lock(&iaxsl[callno]); + if (iax2_lock_callno_unless_destroyed(callno) == 0) { + ast_debug(3, "Hangup initiated on call %d, aborting __send_ping\n", callno); + return; + } - if (iaxs[callno]) { - if (iaxs[callno]->peercallno) { - send_command(iaxs[callno], AST_FRAME_IAX, IAX_COMMAND_PING, 0, NULL, 0, -1); - if (iaxs[callno]->pingid != DONT_RESCHEDULE) { - iaxs[callno]->pingid = iax2_sched_add(sched, ping_time * 1000, send_ping, data); - } - } - } else { - ast_debug(1, "I was supposed to send a PING with callno %d, but no such call exists.\n", callno); + /* Mark pingid as invalid scheduler id. */ + iaxs[callno]->pingid = -1; + + /* callno is now locked. */ + if (iaxs[callno]->peercallno) { + /* Send PING packet. */ + send_command(iaxs[callno], AST_FRAME_IAX, IAX_COMMAND_PING, 0, NULL, 0, -1); + + /* Schedule sending next ping. */ + iaxs[callno]->pingid = iax2_sched_add(sched, ping_time * 1000, send_ping, data); } ast_mutex_unlock(&iaxsl[callno]); } static int send_ping(const void *data) { - int callno = (long) data; - ast_mutex_lock(&iaxsl[callno]); - if (iaxs[callno] && iaxs[callno]->pingid != DONT_RESCHEDULE) { - iaxs[callno]->pingid = -1; - } - ast_mutex_unlock(&iaxsl[callno]); - #ifdef SCHED_MULTITHREADED if (schedule_action(__send_ping, data)) #endif @@ -1743,33 +1761,30 @@ static void __send_lagrq(const void *data) { - int callno = (long) data; + int callno = PTR_TO_CALLNO(data); - ast_mutex_lock(&iaxsl[callno]); + if (iax2_lock_callno_unless_destroyed(callno) == 0) { + ast_debug(3, "Hangup initiated on call %d, aborting __send_lagrq\n", callno); + return; + } - if (iaxs[callno]) { - if (iaxs[callno]->peercallno) { - send_command(iaxs[callno], AST_FRAME_IAX, IAX_COMMAND_LAGRQ, 0, NULL, 0, -1); - if (iaxs[callno]->lagid != DONT_RESCHEDULE) { - iaxs[callno]->lagid = iax2_sched_add(sched, lagrq_time * 1000, send_lagrq, data); - } - } - } else { - ast_debug(1, "I was supposed to send a LAGRQ with callno %d, but no such call exists.\n", callno); + /* Mark lagid as invalid scheduler id. */ + iaxs[callno]->lagid = -1; + + /* callno is now locked. */ + if (iaxs[callno]->peercallno) { + /* Send LAGRQ packet. */ + send_command(iaxs[callno], AST_FRAME_IAX, IAX_COMMAND_LAGRQ, 0, NULL, 0, -1); + + /* Schedule sending next lagrq. */ + iaxs[callno]->lagid = iax2_sched_add(sched, lagrq_time * 1000, send_lagrq, data); } ast_mutex_unlock(&iaxsl[callno]); } static int send_lagrq(const void *data) { - int callno = (long) data; - ast_mutex_lock(&iaxsl[callno]); - if (iaxs[callno] && iaxs[callno]->lagid != DONT_RESCHEDULE) { - iaxs[callno]->lagid = -1; - } - ast_mutex_unlock(&iaxsl[callno]); - #ifdef SCHED_MULTITHREADED if (schedule_action(__send_lagrq, data)) #endif @@ -2053,6 +2068,16 @@ return res; } +/* Call AST_SCHED_DEL on a scheduled task if it is found in scheduler. */ +static int iax2_delete_from_sched(const void* data) +{ + int sched_id = (int)(long)data; + + AST_SCHED_DEL(sched, sched_id); + + return 0; +} + /*!\note Assumes the lock on the pvt is already held, when * iax2_destroy_helper() is called. */ static void iax2_destroy_helper(struct chan_iax2_pvt *pvt) @@ -2069,11 +2094,27 @@ ast_clear_flag64(pvt, IAX_MAXAUTHREQ); } - /* No more pings or lagrq's */ - AST_SCHED_DEL_SPINLOCK(sched, pvt->pingid, &iaxsl[pvt->callno]); - pvt->pingid = DONT_RESCHEDULE; - AST_SCHED_DEL_SPINLOCK(sched, pvt->lagid, &iaxsl[pvt->callno]); - pvt->lagid = DONT_RESCHEDULE; + + + /* Mark call destroy initiated flag. */ + pvt->destroy_initiated = 1; + + /* + * Schedule deleting the scheduled (but didn't run yet) PINGs or LAGRQs. + * Already running tasks will be terminated because of destroy_initiated. + * + * Don't call AST_SCHED_DEL from this thread for pingid and lagid because + * it leads to a deadlock between the scheduler thread callback locking + * the callno mutex and this thread which holds the callno mutex one or + * more times. It is better to have another thread delete the scheduled + * callbacks which doesn't lock the callno mutex. + */ + iax2_sched_add(sched, 0, iax2_delete_from_sched, (void*)(long)pvt->pingid); + iax2_sched_add(sched, 0, iax2_delete_from_sched, (void*)(long)pvt->lagid); + + pvt->pingid = -1; + pvt->lagid = -1; + AST_SCHED_DEL(sched, pvt->autoid); AST_SCHED_DEL(sched, pvt->authid); AST_SCHED_DEL(sched, pvt->initid); Index: main/sched.c =================================================================== diff -u -N -reb9448a1ae18bf16074346bdaae5e0d29d3bcd38 -rcf98c744d5bb7e5f015f201bc79c58462f7aaaed --- a/main/sched.c (.../sched.c) (revision eb9448a1ae18bf16074346bdaae5e0d29d3bcd38) +++ b/main/sched.c (.../sched.c) (revision cf98c744d5bb7e5f015f201bc79c58462f7aaaed) @@ -513,16 +513,8 @@ if (!s && *last_id != id) { ast_debug(1, "Attempted to delete nonexistent schedule entry %d!\n", id); -#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 + /* Removing nonexistent schedule entry shouldn't trigger assert (it was enabled in DEV_MODE); + * because in many places entries is deleted without having valid id. */ *last_id = id; return -1; } else if (!s) {