diff --git a/include/asterisk/sched.h b/include/asterisk/sched.h index 804b05c0c91a8082502a7f903b80ce1802f944cf..7ea6709adb308abfa794abfb44b2cdb746ee278a 100644 --- a/include/asterisk/sched.h +++ b/include/asterisk/sched.h @@ -71,20 +71,24 @@ extern "C" { /*! * \brief schedule task to get deleted and call unref function + * + * Only calls unref function if the delete succeeded. + * * \sa AST_SCHED_DEL * \since 1.6.1 */ #define AST_SCHED_DEL_UNREF(sched, id, refcall) \ do { \ - int _count = 0; \ - while (id > -1 && ast_sched_del(sched, id) && ++_count < 10) { \ + int _count = 0, _id; \ + while ((_id = id) > -1 && ast_sched_del(sched, _id) && ++_count < 10) { \ usleep(1); \ } \ - if (_count == 10) \ - ast_log(LOG_WARNING, "Unable to cancel schedule ID %d. This is probably a bug (%s: %s, line %d).\n", id, __FILE__, __PRETTY_FUNCTION__, __LINE__); \ - if (id > -1) \ + if (_count == 10) { \ + ast_log(LOG_WARNING, "Unable to cancel schedule ID %d. This is probably a bug (%s: %s, line %d).\n", _id, __FILE__, __PRETTY_FUNCTION__, __LINE__); \ + } else if (_id > -1) { \ refcall; \ - id = -1; \ + id = -1; \ + } \ } while (0); /*! diff --git a/main/sched.c b/main/sched.c index d141e7026d99e81198b3a613a18565872eb490a5..e3a7d30ec82b384151aa82a0bea93899acace810 100644 --- a/main/sched.c +++ b/main/sched.c @@ -116,6 +116,8 @@ struct ast_sched_context { struct sched_thread *sched_thread; /*! The scheduled task that is currently executing */ struct sched *currently_executing; + /*! Valid while currently_executing is not NULL */ + pthread_t executing_thread_id; #ifdef SCHED_MAX_CACHE AST_LIST_HEAD_NOLOCK(, sched) schedc; /*!< Cache of unused schedule structures and how many */ @@ -625,15 +627,26 @@ int ast_sched_del(struct ast_sched_context *con, int id) } sched_release(con, s); } else if (con->currently_executing && (id == con->currently_executing->sched_id->id)) { - s = con->currently_executing; - s->deleted = 1; - /* Wait for executing task to complete so that caller of ast_sched_del() does not - * free memory out from under the task. - */ - while (con->currently_executing && (id == con->currently_executing->sched_id->id)) { - ast_cond_wait(&s->cond, &con->lock); + if (con->executing_thread_id == pthread_self()) { + /* The scheduled callback is trying to delete itself. + * Not good as that is a deadlock. */ + ast_log(LOG_ERROR, + "BUG! Trying to delete sched %d from within the callback %p. " + "Ignoring so we don't deadlock\n", + id, con->currently_executing->callback); + ast_log_backtrace(); + /* We'll return -1 below because s is NULL. + * The caller will rightly assume that the unscheduling failed. */ + } else { + s = con->currently_executing; + s->deleted = 1; + /* Wait for executing task to complete so that the caller of + * ast_sched_del() does not free memory out from under the task. */ + while (con->currently_executing && (id == con->currently_executing->sched_id->id)) { + ast_cond_wait(&s->cond, &con->lock); + } + /* Do not sched_release() here because ast_sched_runq() will do it */ } - /* Do not sched_release() here because ast_sched_runq() will do it */ } #ifdef DUMP_SCHEDULER @@ -773,6 +786,7 @@ int ast_sched_runq(struct ast_sched_context *con) */ con->currently_executing = current; + con->executing_thread_id = pthread_self(); ast_mutex_unlock(&con->lock); res = current->callback(current->data); ast_mutex_lock(&con->lock);