[asterisk-commits] kharwell: branch 13 r431179 - in /branches/13: main/ res/ res/res_pjsip/ res/...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Tue Jan 27 13:08:54 CST 2015
Author: kharwell
Date: Tue Jan 27 13:08:44 2015
New Revision: 431179
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=431179
Log:
res_pjsip: make it unloadable (take 2)
Due to the original patch causing memory corruptions it was removed until the
problem could be resolved. This patch is the original patch plus some added
locking around stasis router subcription that was needed to avoid the memory
corruption.
Description of the original problem and patch (still applicable):
The res_pjsip module was previously unloadable. With this patch it can now
be unloaded.
This patch is based off the original patch on the issue (listed below) by Corey
Farrell with a few modifications. Namely, removed a few changes not required to
make the module unloadable and also fixed a bug that would cause asterisk to
crash on unloading.
This patch is the first step (should hopefully be followed by another/others at
some point) in allowing res_pjsip and the modules that depend on it to be
unloadable. At this time, res_pjsip and some of the modules that depend on
res_pjsip cannot be unloaded without causing problems of some sort.
The goal of this patch is to get res_pjsip and only res_pjsip to be able to
unload successfully and/or shutdown without incident (crashes, leaks, etc...).
Other dependent modules may still cause problems on unload.
Basically made sure, with the patch applied, that res_pjsip (with no other
dependent modules loaded) could be succesfully unloaded and Asterisk could
shutdown without any leaks or crashes that pertained directly to res_pjsip.
ASTERISK-24485 #close
Reported by: Corey Farrell
Review: https://reviewboard.asterisk.org/r/4363/
patches:
pjsip_unload-broken-r1.patch submitted by Corey Farrell (license 5909)
Modified:
branches/13/main/stasis_message_router.c
branches/13/res/res_pjsip.c
branches/13/res/res_pjsip/config_auth.c
branches/13/res/res_pjsip/config_transport.c
branches/13/res/res_pjsip/include/res_pjsip_private.h
branches/13/res/res_pjsip/location.c
branches/13/res/res_pjsip/pjsip_configuration.c
branches/13/res/res_pjsip/pjsip_distributor.c
branches/13/res/res_pjsip/pjsip_global_headers.c
branches/13/res/res_pjsip/pjsip_options.c
branches/13/res/res_pjsip/pjsip_outbound_auth.c
Modified: branches/13/main/stasis_message_router.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/main/stasis_message_router.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/main/stasis_message_router.c (original)
+++ branches/13/main/stasis_message_router.c Tue Jan 27 13:08:44 2015
@@ -255,7 +255,9 @@
return;
}
- stasis_unsubscribe(router->subscription);
+ ao2_lock(router);
+ router->subscription = stasis_unsubscribe(router->subscription);
+ ao2_unlock(router);
}
void stasis_message_router_unsubscribe_and_join(
Modified: branches/13/res/res_pjsip.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip.c (original)
+++ branches/13/res/res_pjsip.c Tue Jan 27 13:08:44 2015
@@ -40,6 +40,8 @@
/*** MODULEINFO
<depend>pjproject</depend>
<depend>res_sorcery_config</depend>
+ <depend>res_sorcery_memory</depend>
+ <depend>res_sorcery_astdb</depend>
<support_level>core</support_level>
***/
@@ -1805,7 +1807,7 @@
static struct ast_threadpool *sip_threadpool;
-static int register_service(void *data)
+static int register_service_noref(void *data)
{
pjsip_module **module = data;
if (!ast_pjsip_endpoint) {
@@ -1817,25 +1819,55 @@
return -1;
}
ast_debug(1, "Registered SIP service %.*s (%p)\n", (int) pj_strlen(&(*module)->name), pj_strbuf(&(*module)->name), *module);
- ast_module_ref(ast_module_info->self);
return 0;
}
+static int register_service(void *data)
+{
+ int res;
+
+ if (!(res = register_service_noref(data))) {
+ ast_module_ref(ast_module_info->self);
+ }
+
+ return res;
+}
+
+int internal_sip_register_service(pjsip_module *module)
+{
+ return ast_sip_push_task_synchronous(NULL, register_service_noref, &module);
+}
+
int ast_sip_register_service(pjsip_module *module)
{
return ast_sip_push_task_synchronous(NULL, register_service, &module);
}
-static int unregister_service(void *data)
+static int unregister_service_noref(void *data)
{
pjsip_module **module = data;
- ast_module_unref(ast_module_info->self);
if (!ast_pjsip_endpoint) {
return -1;
}
pjsip_endpt_unregister_module(ast_pjsip_endpoint, *module);
ast_debug(1, "Unregistered SIP service %.*s\n", (int) pj_strlen(&(*module)->name), pj_strbuf(&(*module)->name));
return 0;
+}
+
+static int unregister_service(void *data)
+{
+ int res;
+
+ if (!(res = unregister_service_noref(data))) {
+ ast_module_unref(ast_module_info->self);
+ }
+
+ return res;
+}
+
+int internal_sip_unregister_service(pjsip_module *module)
+{
+ return ast_sip_push_task_synchronous(NULL, unregister_service_noref, &module);
}
void ast_sip_unregister_service(pjsip_module *module)
@@ -1984,26 +2016,38 @@
AST_RWLIST_HEAD_STATIC(endpoint_formatters, ast_sip_endpoint_formatter);
-int ast_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+void internal_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
{
SCOPED_LOCK(lock, &endpoint_formatters, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
AST_RWLIST_INSERT_TAIL(&endpoint_formatters, obj, next);
+}
+
+int ast_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+{
+ internal_sip_register_endpoint_formatter(obj);
ast_module_ref(ast_module_info->self);
return 0;
}
-void ast_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+int internal_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
{
struct ast_sip_endpoint_formatter *i;
SCOPED_LOCK(lock, &endpoint_formatters, AST_RWLIST_WRLOCK, AST_RWLIST_UNLOCK);
AST_RWLIST_TRAVERSE_SAFE_BEGIN(&endpoint_formatters, i, next) {
if (i == obj) {
AST_RWLIST_REMOVE_CURRENT(next);
- ast_module_unref(ast_module_info->self);
break;
}
}
AST_RWLIST_TRAVERSE_SAFE_END;
+ return i == obj ? 0 : -1;
+}
+
+void ast_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj)
+{
+ if (!internal_sip_unregister_endpoint_formatter(obj)) {
+ ast_module_unref(ast_module_info->self);
+ }
}
int ast_sip_format_endpoint_ami(struct ast_sip_endpoint *endpoint,
@@ -3228,7 +3272,7 @@
return AST_MODULE_LOAD_DECLINE;
}
- if (ast_sip_register_service(&supplement_module)) {
+ if (internal_sip_register_service(&supplement_module)) {
ast_log(LOG_ERROR, "Failed to initialize supplement hooks. Aborting load\n");
ast_sip_destroy_distributor();
ast_res_pjsip_destroy_configuration();
@@ -3243,9 +3287,9 @@
return AST_MODULE_LOAD_DECLINE;
}
- if (ast_sip_initialize_outbound_authentication()) {
+ if (internal_sip_initialize_outbound_authentication()) {
ast_log(LOG_ERROR, "Failed to initialize outbound authentication. Aborting load\n");
- ast_sip_unregister_service(&supplement_module);
+ internal_sip_unregister_service(&supplement_module);
ast_sip_destroy_distributor();
ast_res_pjsip_destroy_configuration();
ast_sip_destroy_global_headers();
@@ -3261,8 +3305,6 @@
ast_res_pjsip_init_options_handling(0);
- ast_module_ref(ast_module_info->self);
-
return AST_MODULE_LOAD_SUCCESS;
}
@@ -3276,9 +3318,41 @@
return 0;
}
+static int unload_pjsip(void *data)
+{
+ if (memory_pool) {
+ pj_pool_release(memory_pool);
+ memory_pool = NULL;
+ }
+ if (ast_pjsip_endpoint) {
+ pjsip_endpt_destroy(ast_pjsip_endpoint);
+ ast_pjsip_endpoint = NULL;
+ }
+ pj_caching_pool_destroy(&caching_pool);
+ pj_shutdown();
+ return 0;
+}
+
static int unload_module(void)
{
- /* This will never get called as this module can't be unloaded */
+ ast_res_pjsip_cleanup_options_handling();
+ internal_sip_destroy_outbound_authentication();
+ ast_sip_destroy_distributor();
+ ast_res_pjsip_destroy_configuration();
+ ast_sip_destroy_system();
+ ast_sip_destroy_global_headers();
+ internal_sip_unregister_service(&supplement_module);
+ if (monitor_thread) {
+ stop_monitor_thread();
+ }
+ /* The thread this is called from cannot call PJSIP/PJLIB functions,
+ * so we have to push the work to the threadpool to handle
+ */
+ ast_sip_push_task_synchronous(NULL, unload_pjsip, NULL);
+
+ ast_threadpool_shutdown(sip_threadpool);
+
+ ast_sip_destroy_cli();
return 0;
}
Modified: branches/13/res/res_pjsip/config_auth.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/config_auth.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/config_auth.c (original)
+++ branches/13/res/res_pjsip/config_auth.c Tue Jan 27 13:08:44 2015
@@ -312,7 +312,7 @@
ast_sorcery_object_field_register_custom(sorcery, SIP_SORCERY_AUTH_TYPE, "auth_type",
"userpass", auth_type_handler, auth_type_to_str, NULL, 0, 0);
- ast_sip_register_endpoint_formatter(&endpoint_auth_formatter);
+ internal_sip_register_endpoint_formatter(&endpoint_auth_formatter);
cli_formatter = ao2_alloc(sizeof(struct ast_sip_cli_formatter_entry), NULL);
if (!cli_formatter) {
@@ -337,6 +337,7 @@
{
ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
ast_sip_unregister_cli_formatter(cli_formatter);
-
- return 0;
-}
+ internal_sip_unregister_endpoint_formatter(&endpoint_auth_formatter);
+
+ return 0;
+}
Modified: branches/13/res/res_pjsip/config_transport.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/config_transport.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/config_transport.c (original)
+++ branches/13/res/res_pjsip/config_transport.c Tue Jan 27 13:08:44 2015
@@ -769,7 +769,7 @@
ast_sorcery_object_field_register(sorcery, "transport", "cos", "0", OPT_UINT_T, 0, FLDSET(struct ast_sip_transport, cos));
ast_sorcery_object_field_register(sorcery, "transport", "websocket_write_timeout", AST_DEFAULT_WEBSOCKET_WRITE_TIMEOUT_STR, OPT_INT_T, PARSE_IN_RANGE, FLDSET(struct ast_sip_transport, write_timeout), 1, INT_MAX);
- ast_sip_register_endpoint_formatter(&endpoint_transport_formatter);
+ internal_sip_register_endpoint_formatter(&endpoint_transport_formatter);
cli_formatter = ao2_alloc(sizeof(struct ast_sip_cli_formatter_entry), NULL);
if (!cli_formatter) {
@@ -795,5 +795,7 @@
ast_cli_unregister_multiple(cli_commands, ARRAY_LEN(cli_commands));
ast_sip_unregister_cli_formatter(cli_formatter);
- return 0;
-}
+ internal_sip_unregister_endpoint_formatter(&endpoint_transport_formatter);
+
+ return 0;
+}
Modified: branches/13/res/res_pjsip/include/res_pjsip_private.h
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/include/res_pjsip_private.h?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/include/res_pjsip_private.h (original)
+++ branches/13/res/res_pjsip/include/res_pjsip_private.h Tue Jan 27 13:08:44 2015
@@ -57,7 +57,15 @@
* \retval 0 Success
* \retval non-zero Failure
*/
-int ast_sip_initialize_outbound_authentication(void);
+int internal_sip_initialize_outbound_authentication(void);
+
+/*!
+ * \brief Destroy outbound authentication support
+ *
+ * \retval 0 Success
+ * \retval non-zero Failure
+ */
+void internal_sip_destroy_outbound_authentication(void);
/*!
* \brief Initialize system configuration
@@ -112,4 +120,24 @@
int ast_sip_initialize_cli(void);
void ast_sip_destroy_cli(void);
+/*!
+ * \internal \brief Used by res_pjsip.so to register a service without adding a self reference
+ */
+int internal_sip_register_service(pjsip_module *module);
+
+/*!
+ * \internal \brief Used by res_pjsip.so to unregister a service without removing a self reference
+ */
+int internal_sip_unregister_service(pjsip_module *module);
+
+/*!
+ * \internal \brief Used by res_pjsip.so to register an endpoint formatter without adding a self reference
+ */
+void internal_sip_register_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);
+
+/*!
+ * \internal \brief Used by res_pjsip.so to unregister a endpoint formatter without removing a self reference
+ */
+int internal_sip_unregister_endpoint_formatter(struct ast_sip_endpoint_formatter *obj);
+
#endif /* RES_PJSIP_PRIVATE_H_ */
Modified: branches/13/res/res_pjsip/location.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/location.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/location.c (original)
+++ branches/13/res/res_pjsip/location.c Tue Jan 27 13:08:44 2015
@@ -870,7 +870,7 @@
ast_sorcery_object_field_register(sorcery, "aor", "outbound_proxy", "", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_aor, outbound_proxy));
ast_sorcery_object_field_register(sorcery, "aor", "support_path", "no", OPT_BOOL_T, 1, FLDSET(struct ast_sip_aor, support_path));
- ast_sip_register_endpoint_formatter(&endpoint_aor_formatter);
+ internal_sip_register_endpoint_formatter(&endpoint_aor_formatter);
contact_formatter = ao2_alloc(sizeof(struct ast_sip_cli_formatter_entry), NULL);
if (!contact_formatter) {
@@ -911,6 +911,8 @@
ast_sip_unregister_cli_formatter(contact_formatter);
ast_sip_unregister_cli_formatter(aor_formatter);
- return 0;
-}
-
+ internal_sip_unregister_endpoint_formatter(&endpoint_aor_formatter);
+
+ return 0;
+}
+
Modified: branches/13/res/res_pjsip/pjsip_configuration.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_configuration.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/pjsip_configuration.c (original)
+++ branches/13/res/res_pjsip/pjsip_configuration.c Tue Jan 27 13:08:44 2015
@@ -1670,7 +1670,9 @@
return -1;
}
- ast_sorcery_internal_object_register(sip_sorcery, "nat_hook", sip_nat_hook_alloc, NULL, NULL);
+ if (ast_sorcery_internal_object_register(sip_sorcery, "nat_hook", sip_nat_hook_alloc, NULL, NULL)) {
+ ast_log(LOG_ERROR, "Failed to register nat_hook\n");
+ }
ast_sorcery_object_field_register(sip_sorcery, "endpoint", "type", "", OPT_NOOP_T, 0, 0);
ast_sorcery_object_field_register(sip_sorcery, "endpoint", "context", "default", OPT_STRINGFIELD_T, 0, STRFLDSET(struct ast_sip_endpoint, context));
@@ -1848,6 +1850,7 @@
ast_sip_unregister_cli_formatter(endpoint_formatter);
ast_sip_unregister_cli_formatter(channel_formatter);
ast_sorcery_unref(sip_sorcery);
+ ao2_cleanup(persistent_endpoints);
}
int ast_res_pjsip_reload_configuration(void)
Modified: branches/13/res/res_pjsip/pjsip_distributor.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_distributor.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/pjsip_distributor.c (original)
+++ branches/13/res/res_pjsip/pjsip_distributor.c Tue Jan 27 13:08:44 2015
@@ -21,6 +21,7 @@
#include <pjsip.h>
#include "asterisk/res_pjsip.h"
+#include "include/res_pjsip_private.h"
static int distribute(void *data);
static pj_bool_t distributor(pjsip_rx_data *rdata);
@@ -222,7 +223,7 @@
return artificial_auth;
}
-static struct ast_sip_endpoint *artificial_endpoint;
+static struct ast_sip_endpoint *artificial_endpoint = NULL;
static int create_artificial_endpoint(void)
{
@@ -236,7 +237,7 @@
* the proper size of the vector is returned. This value is
* not actually used anywhere
*/
- AST_VECTOR_APPEND(&artificial_endpoint->inbound_auths, "artificial-auth");
+ AST_VECTOR_APPEND(&artificial_endpoint->inbound_auths, ast_strdup("artificial-auth"));
return 0;
}
@@ -373,13 +374,13 @@
return -1;
}
- if (ast_sip_register_service(&distributor_mod)) {
- return -1;
- }
- if (ast_sip_register_service(&endpoint_mod)) {
- return -1;
- }
- if (ast_sip_register_service(&auth_mod)) {
+ if (internal_sip_register_service(&distributor_mod)) {
+ return -1;
+ }
+ if (internal_sip_register_service(&endpoint_mod)) {
+ return -1;
+ }
+ if (internal_sip_register_service(&auth_mod)) {
return -1;
}
@@ -388,9 +389,9 @@
void ast_sip_destroy_distributor(void)
{
- ast_sip_unregister_service(&distributor_mod);
- ast_sip_unregister_service(&endpoint_mod);
- ast_sip_unregister_service(&auth_mod);
+ internal_sip_unregister_service(&distributor_mod);
+ internal_sip_unregister_service(&endpoint_mod);
+ internal_sip_unregister_service(&auth_mod);
ao2_cleanup(artificial_auth);
ao2_cleanup(artificial_endpoint);
Modified: branches/13/res/res_pjsip/pjsip_global_headers.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_global_headers.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/pjsip_global_headers.c (original)
+++ branches/13/res/res_pjsip/pjsip_global_headers.c Tue Jan 27 13:08:44 2015
@@ -23,6 +23,7 @@
#include "asterisk/res_pjsip.h"
#include "asterisk/linkedlists.h"
+#include "include/res_pjsip_private.h"
static pj_status_t add_request_headers(pjsip_tx_data *tdata);
static pj_status_t add_response_headers(pjsip_tx_data *tdata);
@@ -152,7 +153,7 @@
AST_RWLIST_HEAD_INIT(&request_headers);
AST_RWLIST_HEAD_INIT(&response_headers);
- ast_sip_register_service(&global_header_mod);
+ internal_sip_register_service(&global_header_mod);
}
static void destroy_headers(struct header_list *headers)
@@ -169,4 +170,6 @@
{
destroy_headers(&request_headers);
destroy_headers(&response_headers);
+
+ internal_sip_unregister_service(&global_header_mod);
}
Modified: branches/13/res/res_pjsip/pjsip_options.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_options.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/pjsip_options.c (original)
+++ branches/13/res/res_pjsip/pjsip_options.c Tue Jan 27 13:08:44 2015
@@ -1123,7 +1123,7 @@
return -1;
}
- ast_sip_register_endpoint_formatter(&contact_status_formatter);
+ internal_sip_register_endpoint_formatter(&contact_status_formatter);
ast_manager_register2("PJSIPQualify", EVENT_FLAG_SYSTEM | EVENT_FLAG_REPORTING, ami_sip_qualify, NULL, NULL, NULL);
ast_cli_register_multiple(cli_options, ARRAY_LEN(cli_options));
@@ -1136,7 +1136,7 @@
{
ast_cli_unregister_multiple(cli_options, ARRAY_LEN(cli_options));
ast_manager_unregister("PJSIPQualify");
- ast_sip_unregister_endpoint_formatter(&contact_status_formatter);
+ internal_sip_unregister_endpoint_formatter(&contact_status_formatter);
pjsip_endpt_unregister_module(ast_sip_get_pjsip_endpoint(), &options_module);
ao2_cleanup(sched_qualifies);
Modified: branches/13/res/res_pjsip/pjsip_outbound_auth.c
URL: http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip/pjsip_outbound_auth.c?view=diff&rev=431179&r1=431178&r2=431179
==============================================================================
--- branches/13/res/res_pjsip/pjsip_outbound_auth.c (original)
+++ branches/13/res/res_pjsip/pjsip_outbound_auth.c Tue Jan 27 13:08:44 2015
@@ -91,6 +91,11 @@
return 0;
}
-int ast_sip_initialize_outbound_authentication(void) {
- return ast_sip_register_service(&outbound_auth_mod);
+int internal_sip_initialize_outbound_authentication(void) {
+ return internal_sip_register_service(&outbound_auth_mod);
}
+
+
+void internal_sip_destroy_outbound_authentication(void) {
+ internal_sip_unregister_service(&outbound_auth_mod);
+}
More information about the asterisk-commits
mailing list