Author: mmichelson Date: Tue Aug 26 17:08:39 2014 New Revision: 422070 URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=422070 Log: Fix race condition in the scheduler when deleting a running entry.
When scheduled tasks run, they are removed from the heap (or hashtab). When a scheduled task is deleted, if the task can't be found in the heap (or hashtab), an assertion is triggered. If DO_CRASH is enabled, this assertion causes a crash. The problem is, sometimes it just so happens that someone attempts to delete a scheduled task at the time that it is running, leading to a crash. This change corrects the issue by tracking which task is currently running. If that task is attempted to be deleted, then we mark the task, and then wait for the task to complete. This way, we can be sure to coordinate task deletion and memory freeing. ASTERISK-24212 Reported by Matt Jordan Review: https://reviewboard.asterisk.org/r/3927 Modified: branches/12/main/sched.c Modified: branches/12/main/sched.c URL: http://svnview.digium.com/svn/asterisk/branches/12/main/sched.c?view=diff&rev=422070&r1=422069&r2=422070 ============================================================================== --- branches/12/main/sched.c (original) +++ branches/12/main/sched.c Tue Aug 26 17:08:39 2014 @@ -74,6 +74,13 @@ const void *data; /*!< Data */ ast_sched_cb callback; /*!< Callback */ ssize_t __heap_index; + /*! + * Used to synchronize between thread running a task and thread + * attempting to delete a task + */ + ast_cond_t cond; + /*! Indication that a running task was deleted. */ + unsigned int deleted:1; }; struct sched_thread { @@ -90,6 +97,8 @@ struct ast_hashtab *schedq_ht; /*!< hash table for fast searching */ struct ast_heap *sched_heap; struct sched_thread *sched_thread; + /*! The scheduled task that is currently executing */ + struct sched *currently_executing; #ifdef SCHED_MAX_CACHE AST_LIST_HEAD_NOLOCK(, sched) schedc; /*!< Cache of unused schedule structures and how many */ @@ -229,6 +238,12 @@ return tmp; } +static void sched_free(struct sched *task) +{ + ast_cond_destroy(&task->cond); + ast_free(task); +} + void ast_sched_context_destroy(struct ast_sched_context *con) { struct sched *s; @@ -240,13 +255,13 @@ #ifdef SCHED_MAX_CACHE while ((s = AST_LIST_REMOVE_HEAD(&con->schedc, list))) { - ast_free(s); + sched_free(s); } #endif if (con->sched_heap) { while ((s = ast_heap_pop(con->sched_heap))) { - ast_free(s); + sched_free(s); } ast_heap_destroy(con->sched_heap); con->sched_heap = NULL; @@ -270,11 +285,14 @@ * to minimize the number of necessary malloc()'s */ #ifdef SCHED_MAX_CACHE - if ((tmp = AST_LIST_REMOVE_HEAD(&con->schedc, list))) + if ((tmp = AST_LIST_REMOVE_HEAD(&con->schedc, list))) { con->schedccnt--; - else -#endif + } else +#endif + { tmp = ast_calloc(1, sizeof(*tmp)); + ast_cond_init(&tmp->cond, NULL); + } return tmp; } @@ -292,7 +310,7 @@ con->schedccnt++; } else #endif - ast_free(tmp); + sched_free(tmp); } /*! \brief @@ -462,10 +480,16 @@ if (!ast_hashtab_remove_this_object(con->schedq_ht, s)) { ast_log(LOG_WARNING,"Found sched entry %d, then couldn't remove it?\n", s->id); } - con->schedcnt--; - sched_release(con, s); + } else if (con->currently_executing && (id == con->currently_executing->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. + */ + ast_cond_wait(&s->cond, &con->lock); + /* Do not sched_release() here because ast_sched_runq() will do it */ } #ifdef DUMP_SCHEDULER @@ -610,11 +634,14 @@ * should return 0. */ + con->currently_executing = current; ast_mutex_unlock(&con->lock); res = current->callback(current->data); ast_mutex_lock(&con->lock); - - if (res) { + con->currently_executing = NULL; + ast_cond_signal(¤t->cond); + + if (res && !current->deleted) { /* * If they return non-zero, we should schedule them to be * run again. -- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- svn-commits mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/svn-commits
