[svn-commits] rmudgett: branch 1.8 r356677 - in /branches/1.8: channels/ channels/sip/inclu...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Fri Feb 24 12:23:33 CST 2012
Author: rmudgett
Date: Fri Feb 24 12:23:28 2012
New Revision: 356677
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=356677
Log:
Fix worker thread resource leak in SIP TCP/TLS.
The SIP TCP/TLS worker threads were created joinable but noone could join
them if they died on their own.
* Fix the SIP TCP/TLS worker threads to not be created joinable.
* _sip_tcp_helper_thread() only needs one parameter since the pvt
parameter is only passed in as NULL and never used.
(closes issue ASTERISK-19203)
Reported by: Steve Davies
Review: https://reviewboard.asterisk.org/r/1714/
Modified:
branches/1.8/channels/chan_sip.c
branches/1.8/channels/sip/include/sip.h
branches/1.8/include/asterisk/tcptls.h
Modified: branches/1.8/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/chan_sip.c?view=diff&rev=356677&r1=356676&r2=356677
==============================================================================
--- branches/1.8/channels/chan_sip.c (original)
+++ branches/1.8/channels/chan_sip.c Fri Feb 24 12:23:28 2012
@@ -1496,7 +1496,7 @@
static void get_realm(struct sip_pvt *p, const struct sip_request *req);
/*-- TCP connection handling ---*/
-static void *_sip_tcp_helper_thread(struct sip_pvt *pvt, struct ast_tcptls_session_instance *tcptls_session);
+static void *_sip_tcp_helper_thread(struct ast_tcptls_session_instance *tcptls_session);
static void *sip_tcp_worker_fn(void *);
/*--- Constructing requests and responses */
@@ -2468,7 +2468,7 @@
{
struct ast_tcptls_session_instance *tcptls_session = data;
- return _sip_tcp_helper_thread(NULL, tcptls_session);
+ return _sip_tcp_helper_thread(tcptls_session);
}
/*! \brief Check if the authtimeout has expired.
@@ -2499,7 +2499,7 @@
/*! \brief SIP TCP thread management function
This function reads from the socket, parses the packet into a request
*/
-static void *_sip_tcp_helper_thread(struct sip_pvt *pvt, struct ast_tcptls_session_instance *tcptls_session)
+static void *_sip_tcp_helper_thread(struct ast_tcptls_session_instance *tcptls_session)
{
int res, cl, timeout = -1, authenticated = 0, flags, after_poll = 0, need_poll = 1;
time_t start;
@@ -25418,6 +25418,7 @@
struct ast_tcptls_session_instance *tcptls_session;
struct ast_tcptls_session_args *ca;
struct ast_sockaddr sa_tmp;
+ pthread_t launched;
/* check to see if a socket is already active */
if ((s->fd != -1) && (s->type == SIP_TRANSPORT_UDP)) {
@@ -25442,7 +25443,7 @@
}
/* At this point we are dealing with a TCP/TLS connection
- * 1. We need to check to see if a connectin thread exists
+ * 1. We need to check to see if a connection thread exists
* for this address, if so use that.
* 2. If a thread does not exist for this address, but the tcptls_session
* exists on the socket, the connection was closed.
@@ -25513,7 +25514,7 @@
/* Give the new thread a reference to the tcptls_session */
ao2_ref(s->tcptls_session, +1);
- if (ast_pthread_create_background(&ca->master, NULL, sip_tcp_worker_fn, s->tcptls_session)) {
+ if (ast_pthread_create_detached_background(&launched, NULL, sip_tcp_worker_fn, s->tcptls_session)) {
ast_debug(1, "Unable to launch '%s'.", ca->name);
ao2_ref(s->tcptls_session, -1); /* take away the thread ref we just gave it */
goto create_tcptls_session_fail;
@@ -30369,6 +30370,7 @@
struct sip_threadinfo *th;
struct ast_context *con;
struct ao2_iterator i;
+ int wait_count;
network_change_event_unsubscribe();
@@ -30425,7 +30427,6 @@
pthread_t thread = th->threadid;
th->stop = 1;
pthread_kill(thread, SIGURG);
- pthread_join(thread, NULL);
ao2_t_ref(th, -1, "decrement ref from iterator");
}
ao2_iterator_destroy(&i);
@@ -30471,21 +30472,11 @@
sip_epa_unregister_all();
destroy_escs();
- if (default_tls_cfg.certfile) {
- ast_free(default_tls_cfg.certfile);
- }
- if (default_tls_cfg.pvtfile) {
- ast_free(default_tls_cfg.pvtfile);
- }
- if (default_tls_cfg.cipher) {
- ast_free(default_tls_cfg.cipher);
- }
- if (default_tls_cfg.cafile) {
- ast_free(default_tls_cfg.cafile);
- }
- if (default_tls_cfg.capath) {
- ast_free(default_tls_cfg.capath);
- }
+ ast_free(default_tls_cfg.certfile);
+ ast_free(default_tls_cfg.pvtfile);
+ ast_free(default_tls_cfg.cipher);
+ ast_free(default_tls_cfg.cafile);
+ ast_free(default_tls_cfg.capath);
cleanup_all_regs();
ASTOBJ_CONTAINER_DESTROYALL(®l, sip_registry_destroy);
@@ -30502,6 +30493,21 @@
} while(0));
ASTOBJ_CONTAINER_DESTROYALL(&submwil, sip_subscribe_mwi_destroy);
ASTOBJ_CONTAINER_DESTROY(&submwil);
+
+ /*
+ * Wait awhile for the TCP/TLS thread container to become empty.
+ *
+ * XXX This is a hack, but the worker threads cannot be created
+ * joinable. They can die on their own and remove themselves
+ * from the container thus resulting in a huge memory leak.
+ */
+ wait_count = 1000;
+ while (ao2_container_count(threadt) && --wait_count) {
+ sched_yield();
+ }
+ if (!wait_count) {
+ ast_debug(2, "TCP/TLS thread container did not become empty :(\n");
+ }
ao2_t_ref(peers, -1, "unref the peers table");
ao2_t_ref(peers_by_ip, -1, "unref the peers_by_ip table");
Modified: branches/1.8/channels/sip/include/sip.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/channels/sip/include/sip.h?view=diff&rev=356677&r1=356676&r2=356677
==============================================================================
--- branches/1.8/channels/sip/include/sip.h (original)
+++ branches/1.8/channels/sip/include/sip.h Fri Feb 24 12:23:28 2012
@@ -1327,6 +1327,7 @@
};
/*! \brief Definition of a thread that handles a socket */
struct sip_threadinfo {
+ /*! TRUE if the thread needs to kill itself. (The module is being unloaded.) */
int stop;
int alert_pipe[2]; /*! Used to alert tcptls thread when packet is ready to be written */
pthread_t threadid;
Modified: branches/1.8/include/asterisk/tcptls.h
URL: http://svnview.digium.com/svn/asterisk/branches/1.8/include/asterisk/tcptls.h?view=diff&rev=356677&r1=356676&r2=356677
==============================================================================
--- branches/1.8/include/asterisk/tcptls.h (original)
+++ branches/1.8/include/asterisk/tcptls.h Fri Feb 24 12:23:28 2012
@@ -128,6 +128,7 @@
struct ast_tls_config *tls_cfg; /*!< points to the SSL configuration if any */
int accept_fd;
int poll_timeout;
+ /*! Server accept_fn thread ID used for external shutdown requests. */
pthread_t master;
void *(*accept_fn)(void *); /*!< the function in charge of doing the accept */
void (*periodic_fn)(void *);/*!< something we may want to run before after select on the accept socket */
@@ -146,6 +147,7 @@
int client;
struct ast_sockaddr remote_address;
struct ast_tcptls_session_args *parent;
+ /*! XXX Why do we still use this lock when this struct is allocated as an ao2 object which has its own lock? */
ast_mutex_t lock;
};
More information about the svn-commits
mailing list