From 03e89247ded06b5320684bfa10a6e5e2fbe57646 Mon Sep 17 00:00:00 2001 From: Mark Michelson Date: Tue, 15 Jan 2013 20:15:00 +0000 Subject: [PATCH] Address further review feedback from David Lee. * Clarify some documentation * Change copyright date of taskprocessor files * Address potential issue of creating taskprocessor with listener if taskprocessor with that name exists already git-svn-id: https://origsvn.digium.com/svn/asterisk/team/mmichelson/threadpool@379124 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- include/asterisk/taskprocessor.h | 4 +- include/asterisk/threadpool.h | 2 +- main/taskprocessor.c | 87 ++++++++++++++++++-------------- main/threadpool.c | 4 +- 4 files changed, 55 insertions(+), 42 deletions(-) diff --git a/include/asterisk/taskprocessor.h b/include/asterisk/taskprocessor.h index c64a8f9028..eb3137c964 100644 --- a/include/asterisk/taskprocessor.h +++ b/include/asterisk/taskprocessor.h @@ -1,7 +1,7 @@ /* * Asterisk -- An open source telephony toolkit. * - * Copyright (C) 2007-2008, Digium, Inc. + * Copyright (C) 2007-2013, Digium, Inc. * * Dwayne M. Hubbard * @@ -22,7 +22,7 @@ * * \author Dwayne M. Hubbard * - * \note A taskprocessor is a named singleton containing a task queue that + * \note A taskprocessor is a named object containing a task queue that * serializes tasks pushed into it by [a] module(s) that reference the taskprocessor. * A taskprocessor is created the first time its name is requested via the * ast_taskprocessor_get() function or the ast_taskprocessor_create_with_listener() diff --git a/include/asterisk/threadpool.h b/include/asterisk/threadpool.h index a549ae3e15..f003ace1f1 100644 --- a/include/asterisk/threadpool.h +++ b/include/asterisk/threadpool.h @@ -135,7 +135,7 @@ struct ast_threadpool_listener *ast_threadpool_listener_alloc( * in and will be automatically acted upon by threads within the pool. * * \param name The name for the threadpool - * \param listener The listener the threadpool will notify of changes + * \param listener The listener the threadpool will notify of changes. Can be NULL. * \param options The behavioral options for this threadpool * \retval NULL Failed to create the threadpool * \retval non-NULL The newly-created threadpool diff --git a/main/taskprocessor.c b/main/taskprocessor.c index 911eb76f8d..fae93967ae 100644 --- a/main/taskprocessor.c +++ b/main/taskprocessor.c @@ -1,7 +1,7 @@ /* * Asterisk -- An open source telephony toolkit. * - * Copyright (C) 2007-2008, Digium, Inc. + * Copyright (C) 2007-2013, Digium, Inc. * * Dwayne M. Hubbard * @@ -487,6 +487,50 @@ static void *default_listener_pvt_alloc(void) return pvt; } +static struct ast_taskprocessor *__allocate_taskprocessor(const char *name, struct ast_taskprocessor_listener *listener) +{ + RAII_VAR(struct ast_taskprocessor *, p, + ao2_alloc(sizeof(*p), tps_taskprocessor_destroy), ao2_cleanup); + + if (!p) { + ast_log(LOG_WARNING, "failed to create taskprocessor '%s'\n", name); + return NULL; + } + + if (!(p->stats = ast_calloc(1, sizeof(*p->stats)))) { + ast_log(LOG_WARNING, "failed to create taskprocessor stats for '%s'\n", name); + return NULL; + } + if (!(p->name = ast_strdup(name))) { + ao2_ref(p, -1); + return NULL; + } + + ao2_ref(listener, +1); + p->listener = listener; + + ao2_ref(p, +1); + listener->tps = p; + + if (!(ao2_link(tps_singletons, p))) { + ast_log(LOG_ERROR, "Failed to add taskprocessor '%s' to container\n", p->name); + return NULL; + } + + if (p->listener->callbacks->start(p->listener)) { + ast_log(LOG_ERROR, "Unable to start taskprocessor listener for taskprocessor %s\n", p->name); + ast_taskprocessor_unreference(p); + return NULL; + } + + /* RAII_VAR will decrement the refcount at the end of the function. + * Since we want to pass back a reference to p, we bump the refcount + */ + ao2_ref(p, +1); + return p; + +} + /* Provide a reference to a taskprocessor. Create the taskprocessor if necessary, but don't * create the taskprocessor if we were told via ast_tps_options to return a reference only * if it already exists */ @@ -519,7 +563,7 @@ struct ast_taskprocessor *ast_taskprocessor_get(const char *name, enum ast_tps_o return NULL; } - p = ast_taskprocessor_create_with_listener(name, listener); + p = __allocate_taskprocessor(name, listener); if (!p) { default_listener_pvt_destroy(pvt); ao2_ref(listener, -1); @@ -534,46 +578,13 @@ struct ast_taskprocessor *ast_taskprocessor_get(const char *name, enum ast_tps_o struct ast_taskprocessor *ast_taskprocessor_create_with_listener(const char *name, struct ast_taskprocessor_listener *listener) { - RAII_VAR(struct ast_taskprocessor *, p, - ao2_alloc(sizeof(*p), tps_taskprocessor_destroy), - ao2_cleanup); - - if (!p) { - ast_log(LOG_WARNING, "failed to create taskprocessor '%s'\n", name); - return NULL; - } - - if (!(p->stats = ast_calloc(1, sizeof(*p->stats)))) { - ast_log(LOG_WARNING, "failed to create taskprocessor stats for '%s'\n", name); - return NULL; - } - if (!(p->name = ast_strdup(name))) { - ao2_ref(p, -1); - return NULL; - } - - ao2_ref(listener, +1); - p->listener = listener; - - ao2_ref(p, +1); - listener->tps = p; - - if (!(ao2_link(tps_singletons, p))) { - ast_log(LOG_ERROR, "Failed to add taskprocessor '%s' to container\n", p->name); - return NULL; - } + struct ast_taskprocessor *p = ao2_find(tps_singletons, name, OBJ_KEY); - if (p->listener->callbacks->start(p->listener)) { - ast_log(LOG_ERROR, "Unable to start taskprocessor listener for taskprocessor %s\n", p->name); + if (p) { ast_taskprocessor_unreference(p); return NULL; } - - /* RAII_VAR will decrement the refcount at the end of the function. - * Since we want to pass back a reference to p, we bump the refcount - */ - ao2_ref(p, +1); - return p; + return __allocate_taskprocessor(name, listener); } /* decrement the taskprocessor reference count and unlink from the container if necessary */ diff --git a/main/threadpool.c b/main/threadpool.c index 358184caad..bdb71dce5a 100644 --- a/main/threadpool.c +++ b/main/threadpool.c @@ -613,7 +613,9 @@ static void threadpool_tps_shutdown(struct ast_taskprocessor_listener *listener) { struct ast_threadpool *pool = listener->user_data; - pool->listener->callbacks->shutdown(pool->listener); + if (pool->listener && pool->listener->callbacks->shutdown) { + pool->listener->callbacks->shutdown(pool->listener); + } ao2_cleanup(pool->active_threads); ao2_cleanup(pool->idle_threads); ao2_cleanup(pool->zombie_threads);