[svn-commits] bbryant: branch 1.6.0 r123547 - in /branches/1.6.0: ./ channels/ include/aste...
SVN commits to the Digium repositories
svn-commits at lists.digium.com
Tue Jun 17 16:57:16 CDT 2008
Author: bbryant
Date: Tue Jun 17 16:57:15 2008
New Revision: 123547
URL: http://svn.digium.com/view/asterisk?view=rev&rev=123547
Log:
Merged revisions 123546 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk
........
r123546 | bbryant | 2008-06-17 16:46:57 -0500 (Tue, 17 Jun 2008) | 5 lines
Updates all usages of ast_tcptls_session_instance to be managed by reference counts so that they only get destroyed when all threads are done using
them, and memory does not get free'd causing strange issues with SIP.
This code was originally written by russellb in the team/group/issue_11972/ branch.
........
Modified:
branches/1.6.0/ (props changed)
branches/1.6.0/channels/chan_sip.c
branches/1.6.0/include/asterisk/tcptls.h
branches/1.6.0/main/http.c
branches/1.6.0/main/manager.c
branches/1.6.0/main/tcptls.c
Propchange: branches/1.6.0/
------------------------------------------------------------------------------
Binary property 'trunk-merged' - no diff available.
Modified: branches/1.6.0/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/channels/chan_sip.c?view=diff&rev=123547&r1=123546&r2=123547
==============================================================================
--- branches/1.6.0/channels/chan_sip.c (original)
+++ branches/1.6.0/channels/chan_sip.c Tue Jun 17 16:57:15 2008
@@ -775,7 +775,6 @@
/*!< The SIP socket definition */
struct sip_socket {
- ast_mutex_t *lock;
enum sip_transport type;
int fd;
uint16_t port;
@@ -821,6 +820,7 @@
char *header[SIP_MAX_HEADERS];
char *line[SIP_MAX_LINES];
char data[SIP_MAX_PACKET];
+ /* XXX Do we need to unref socket.ser when the request goes away? */
struct sip_socket socket; /*!< The socket used for this request */
};
@@ -2142,14 +2142,6 @@
static void *_sip_tcp_helper_thread(struct sip_pvt *pvt, struct ast_tcptls_session_instance *ser);
-static void *sip_tcp_helper_thread(void *data)
-{
- struct sip_pvt *pvt = data;
- struct ast_tcptls_session_instance *ser = pvt->socket.ser;
-
- return _sip_tcp_helper_thread(pvt, ser);
-}
-
static void *sip_tcp_worker_fn(void *data)
{
struct ast_tcptls_session_instance *ser = data;
@@ -2163,7 +2155,7 @@
int res, cl;
struct sip_request req = { 0, } , reqcpy = { 0, };
struct sip_threadinfo *me;
- char buf[1024];
+ char buf[1024] = "";
me = ast_calloc(1, sizeof(*me));
@@ -2181,12 +2173,6 @@
AST_LIST_INSERT_TAIL(&threadl, me, list);
AST_LIST_UNLOCK(&threadl);
- req.socket.lock = ast_calloc(1, sizeof(*req.socket.lock));
-
- if (!req.socket.lock)
- goto cleanup;
-
- ast_mutex_init(req.socket.lock);
for (;;) {
memset(req.data, 0, sizeof(req.data));
@@ -2210,14 +2196,12 @@
/* Read in headers one line at a time */
while (req.len < 4 || strncmp((char *)&req.data + req.len - 4, "\r\n\r\n", 4)) {
- if (req.socket.lock)
- ast_mutex_lock(req.socket.lock);
+ ast_mutex_lock(&ser->lock);
if (!fgets(buf, sizeof(buf), ser->f)) {
- ast_mutex_unlock(req.socket.lock);
+ ast_mutex_unlock(&ser->lock);
goto cleanup;
}
- if (req.socket.lock)
- ast_mutex_unlock(req.socket.lock);
+ ast_mutex_unlock(&ser->lock);
if (me->stop)
goto cleanup;
strncat(req.data, buf, sizeof(req.data) - req.len - 1);
@@ -2226,12 +2210,12 @@
parse_copy(&reqcpy, &req);
if (sscanf(get_header(&reqcpy, "Content-Length"), "%d", &cl)) {
while (cl > 0) {
- if (req.socket.lock)
- ast_mutex_lock(req.socket.lock);
- if (!fread(buf, (cl < sizeof(buf)) ? cl : sizeof(buf), 1, ser->f))
+ ast_mutex_lock(&ser->lock);
+ if (!fread(buf, (cl < sizeof(buf)) ? cl : sizeof(buf), 1, ser->f)) {
+ ast_mutex_unlock(&ser->lock);
goto cleanup;
- if (req.socket.lock)
- ast_mutex_unlock(req.socket.lock);
+ }
+ ast_mutex_unlock(&ser->lock);
if (me->stop)
goto cleanup;
cl -= strlen(buf);
@@ -2250,13 +2234,11 @@
ast_free(me);
cleanup2:
fclose(ser->f);
- ser = ast_tcptls_session_instance_destroy(ser);
-
- if (req.socket.lock) {
- ast_mutex_destroy(req.socket.lock);
- ast_free(req.socket.lock);
- req.socket.lock = NULL;
- }
+ ser->f = NULL;
+ ser->fd = -1;
+
+ ao2_ref(ser, -1);
+ ser = NULL;
return NULL;
}
@@ -2532,8 +2514,8 @@
if (sip_prepare_socket(p) < 0)
return XMIT_ERROR;
- if (p->socket.lock)
- ast_mutex_lock(p->socket.lock);
+ if (p->socket.ser)
+ ast_mutex_lock(&p->socket.ser->lock);
if (p->socket.type & SIP_TRANSPORT_UDP)
res = sendto(p->socket.fd, data, len, 0, (const struct sockaddr *)dst, sizeof(struct sockaddr_in));
@@ -2544,8 +2526,8 @@
ast_debug(1, "No p->socket.ser->f len=%d\n", len);
}
- if (p->socket.lock)
- ast_mutex_unlock(p->socket.lock);
+ if (p->socket.ser)
+ ast_mutex_unlock(&p->socket.ser->lock);
if (res == -1) {
switch (errno) {
@@ -3513,7 +3495,11 @@
if (peer->dnsmgr)
ast_dnsmgr_release(peer->dnsmgr);
clear_peer_mailboxes(peer);
- ast_free(peer);
+
+ if (peer->socket.ser) {
+ ao2_ref(peer->socket.ser, -1);
+ peer->socket.ser = NULL;
+ }
}
/*! \brief Update peer data in database (if used) */
@@ -3908,6 +3894,20 @@
}
}
+static void copy_socket_data(struct sip_socket *to_sock, const struct sip_socket *from_sock)
+{
+ if (to_sock->ser) {
+ ao2_ref(to_sock->ser, -1);
+ to_sock->ser = NULL;
+ }
+
+ if (from_sock->ser) {
+ ao2_ref(from_sock->ser, +1);
+ }
+
+ *to_sock = *from_sock;
+}
+
/*! \brief Create address structure from peer reference.
* This function copies data from peer to the dialog, so we don't have to look up the peer
* again from memory or database during the life time of the dialog.
@@ -3916,7 +3916,7 @@
*/
static int create_addr_from_peer(struct sip_pvt *dialog, struct sip_peer *peer)
{
- dialog->socket = peer->socket;
+ copy_socket_data(&dialog->socket, &peer->socket);
if ((peer->addr.sin_addr.s_addr || peer->defaddr.sin_addr.s_addr) &&
(!peer->maxms || ((peer->lastms >= 0) && (peer->lastms <= peer->maxms)))) {
@@ -4369,6 +4369,11 @@
ast_mutex_destroy(&p->pvt_lock);
ast_string_field_free_memory(p);
+
+ if (p->socket.ser) {
+ ao2_ref(p->socket.ser, -1);
+ p->socket.ser = NULL;
+ }
ast_free(p);
return 0;
@@ -7574,11 +7579,7 @@
build_via(p);
ast_string_field_set(p, callid, callid);
- p->socket.lock = req->socket.lock;
- p->socket.type = req->socket.type;
- p->socket.fd = req->socket.fd;
- p->socket.port = req->socket.port;
- p->socket.ser = req->socket.ser;
+ copy_socket_data(&p->socket, &req->socket);
/* Use this temporary pvt structure to send the message */
__transmit_response(p, msg, req, XMIT_UNRELIABLE);
@@ -9798,7 +9799,8 @@
}
}
- pvt->socket = peer->socket = req->socket;
+ copy_socket_data(&peer->socket, &req->socket);
+ copy_socket_data(&pvt->socket, &peer->socket);
/* Look for brackets */
curi = contact;
@@ -18101,7 +18103,6 @@
req.socket.type = SIP_TRANSPORT_UDP;
req.socket.ser = NULL;
req.socket.port = bindaddr.sin_port;
- req.socket.lock = NULL;
handle_request_do(&req, &sin);
@@ -18150,7 +18151,7 @@
return 1;
}
- p->socket = req->socket;
+ copy_socket_data(&p->socket, &req->socket);
/* Go ahead and lock the owner if it has one -- we may need it */
/* becaues this is deadlock-prone, we need to try and unlock if failed */
@@ -18245,13 +18246,18 @@
if ((ser = sip_tcp_locate(&ca.sin))) {
s->fd = ser->fd;
+ if (s->ser) {
+ ao2_ref(s->ser, -1);
+ s->ser = NULL;
+ }
+ ao2_ref(ser, +1);
s->ser = ser;
return s->fd;
}
- if (s->ser && s->ser->parent->tls_cfg)
+ if (s->ser && s->ser->parent->tls_cfg) {
ca.tls_cfg = s->ser->parent->tls_cfg;
- else {
+ } else {
if (s->type & SIP_TRANSPORT_TLS) {
ca.tls_cfg = ast_calloc(1, sizeof(*ca.tls_cfg));
if (!ca.tls_cfg)
@@ -18261,7 +18267,12 @@
ast_copy_string(ca.hostname, p->tohost, sizeof(ca.hostname));
}
}
- s->ser = (!s->ser) ? ast_tcptls_client_start(&ca) : s->ser;
+
+ if (s->ser) {
+ /* the pvt socket already has a server instance ... */
+ } else {
+ s->ser = ast_tcptls_client_start(&ca);
+ }
if (!s->ser) {
if (ca.tls_cfg)
@@ -18271,8 +18282,12 @@
s->fd = ca.accept_fd;
- if (ast_pthread_create_background(&ca.master, NULL, sip_tcp_helper_thread, p)) {
+ /* Give the new thread a reference */
+ ao2_ref(s->ser, +1);
+
+ if (ast_pthread_create_background(&ca.master, NULL, sip_tcp_worker_fn, s->ser)) {
ast_debug(1, "Unable to launch '%s'.", ca.name);
+ ao2_ref(s->ser, -1);
close(ca.accept_fd);
s->fd = ca.accept_fd = -1;
}
Modified: branches/1.6.0/include/asterisk/tcptls.h
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/include/asterisk/tcptls.h?view=diff&rev=123547&r1=123546&r2=123547
==============================================================================
--- branches/1.6.0/include/asterisk/tcptls.h (original)
+++ branches/1.6.0/include/asterisk/tcptls.h Tue Jun 17 16:57:15 2008
@@ -50,6 +50,7 @@
#define _ASTERISK_SERVER_H
#include "asterisk/utils.h"
+#include "asterisk/astobj2.h"
#if defined(HAVE_OPENSSL) && (defined(HAVE_FUNOPEN) || defined(HAVE_FOPENCOOKIE))
#define DO_SSL /* comment in/out if you want to support ssl */
@@ -127,6 +128,7 @@
int client;
struct sockaddr_in requestor;
struct server_args *parent;
+ ast_mutex_t lock;
};
/*! \brief
@@ -166,11 +168,4 @@
HOOK_T ast_tcptls_server_read(struct ast_tcptls_session_instance *ser, void *buf, size_t count);
HOOK_T ast_tcptls_server_write(struct ast_tcptls_session_instance *ser, void *buf, size_t count);
-/*!
- * \brief Destroy a server instance
- *
- * \return NULL for convenience
- */
-struct ast_tcptls_session_instance *ast_tcptls_session_instance_destroy(struct ast_tcptls_session_instance *i);
-
#endif /* _ASTERISK_SERVER_H */
Modified: branches/1.6.0/main/http.c
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/main/http.c?view=diff&rev=123547&r1=123546&r2=123547
==============================================================================
--- branches/1.6.0/main/http.c (original)
+++ branches/1.6.0/main/http.c Tue Jun 17 16:57:15 2008
@@ -868,7 +868,8 @@
done:
fclose(ser->f);
- ser = ast_tcptls_session_instance_destroy(ser);
+ ao2_ref(ser, -1);
+ ser = NULL;
return NULL;
}
Modified: branches/1.6.0/main/manager.c
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/main/manager.c?view=diff&rev=123547&r1=123546&r2=123547
==============================================================================
--- branches/1.6.0/main/manager.c (original)
+++ branches/1.6.0/main/manager.c Tue Jun 17 16:57:15 2008
@@ -2937,7 +2937,8 @@
destroy_session(s);
done:
- ser = ast_tcptls_session_instance_destroy(ser);
+ ao2_ref(ser, -1);
+ ser = NULL;
return NULL;
}
Modified: branches/1.6.0/main/tcptls.c
URL: http://svn.digium.com/view/asterisk/branches/1.6.0/main/tcptls.c?view=diff&rev=123547&r1=123546&r2=123547
==============================================================================
--- branches/1.6.0/main/tcptls.c (original)
+++ branches/1.6.0/main/tcptls.c Tue Jun 17 16:57:15 2008
@@ -83,6 +83,12 @@
HOOK_T ast_tcptls_server_read(struct ast_tcptls_session_instance *ser, void *buf, size_t count)
{
+ if (ser->fd == -1) {
+ ast_log(LOG_ERROR, "server_read called with an fd of -1\n");
+ errno = EIO;
+ return -1;
+ }
+
#ifdef DO_SSL
if (ser->ssl)
return ssl_read(ser->ssl, buf, count);
@@ -92,11 +98,23 @@
HOOK_T ast_tcptls_server_write(struct ast_tcptls_session_instance *ser, void *buf, size_t count)
{
+ if (ser->fd == -1) {
+ ast_log(LOG_ERROR, "server_write called with an fd of -1\n");
+ errno = EIO;
+ return -1;
+ }
+
#ifdef DO_SSL
if (ser->ssl)
return ssl_write(ser->ssl, buf, count);
#endif
return write(ser->fd, buf, count);
+}
+
+static void session_instance_destructor(void *obj)
+{
+ struct ast_tcptls_session_instance *i = obj;
+ ast_mutex_destroy(&i->lock);
}
void *ast_tcptls_server_root(void *data)
@@ -123,12 +141,15 @@
ast_log(LOG_WARNING, "Accept failed: %s\n", strerror(errno));
continue;
}
- ser = ast_calloc(1, sizeof(*ser));
+ ser = ao2_alloc(sizeof(*ser), session_instance_destructor);
if (!ser) {
ast_log(LOG_WARNING, "No memory for new session: %s\n", strerror(errno));
close(fd);
continue;
}
+
+ ast_mutex_init(&ser->lock);
+
flags = fcntl(fd, F_GETFL);
fcntl(fd, F_SETFL, flags & ~O_NONBLOCK);
ser->fd = fd;
@@ -140,7 +161,7 @@
if (ast_pthread_create_detached_background(&launched, NULL, ast_make_file_from_fd, ser)) {
ast_log(LOG_WARNING, "Unable to launch helper thread: %s\n", strerror(errno));
close(ser->fd);
- ast_free(ser);
+ ao2_ref(ser, -1);
}
}
return NULL;
@@ -235,8 +256,10 @@
goto error;
}
- if (!(ser = ast_calloc(1, sizeof(*ser))))
- goto error;
+ if (!(ser = ao2_alloc(sizeof(*ser), session_instance_destructor)))
+ goto error;
+
+ ast_mutex_init(&ser->lock);
flags = fcntl(desc->accept_fd, F_GETFL);
fcntl(desc->accept_fd, F_SETFL, flags & ~O_NONBLOCK);
@@ -262,7 +285,7 @@
close(desc->accept_fd);
desc->accept_fd = -1;
if (ser)
- ast_free(ser);
+ ao2_ref(ser, -1);
return NULL;
}
@@ -447,8 +470,3 @@
return ser;
}
-struct ast_tcptls_session_instance *ast_tcptls_session_instance_destroy(struct ast_tcptls_session_instance *i)
-{
- ast_free(i);
- return NULL;
-}
More information about the svn-commits
mailing list