Updates all usages of ast_tcptls_session_instance to be managed by reference counts...

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.

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@123546 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/apps/app_externalivr.c b/apps/app_externalivr.c
index 460c3cf..e658357 100644 (file)
--- a/apps/app_externalivr.c
+++ b/apps/app_externalivr.c
@@ -515,8 +515,7 @@ static int app_exec(struct ast_channel *chan, void *data)
        if (child_stderr[1])
                close(child_stderr[1]);
        if (ser) {
-               fclose(ser->f);
-               ast_tcptls_session_instance_destroy(ser);
+               ao2_ref(ser, -1);
        }
        while ((entry = AST_LIST_REMOVE_HEAD(&u->playlist, list)))
                ast_free(entry);

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 0c4a6e3..d6a1ec9 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -798,7 +798,6 @@ enum sip_transport {
 
 /*!< The SIP socket definition */
 struct sip_socket {
-       ast_mutex_t *lock;
        enum sip_transport type;
        int fd;
        uint16_t port;
@@ -844,6 +843,7 @@ struct sip_request {
        char *header[SIP_MAX_HEADERS];
        char *line[SIP_MAX_LINES];
        struct ast_str *data;   
+       /* XXX Do we need to unref socket.ser when the request goes away? */
        struct sip_socket socket;       /*!< The socket used for this request */
 };
 
@@ -2291,14 +2291,6 @@ static struct ast_rtp_protocol sip_rtp = {
 
 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;
@@ -2312,7 +2304,7 @@ static void *_sip_tcp_helper_thread(struct sip_pvt *pvt, struct ast_tcptls_sessi
        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));
 
@@ -2330,12 +2322,6 @@ static void *_sip_tcp_helper_thread(struct sip_pvt *pvt, struct ast_tcptls_sessi
        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);
        if (!(req.data = ast_str_create(SIP_MIN_PACKET)))
                goto cleanup;
        if (!(reqcpy.data = ast_str_create(SIP_MIN_PACKET)))
@@ -2364,14 +2350,12 @@ static void *_sip_tcp_helper_thread(struct sip_pvt *pvt, struct ast_tcptls_sessi
 
                /* Read in headers one line at a time */
                while (req.len < 4 || strncmp((char *)&req.data->str + 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;
                        ast_str_append(&req.data, 0, "%s", buf);
@@ -2381,12 +2365,12 @@ static void *_sip_tcp_helper_thread(struct sip_pvt *pvt, struct ast_tcptls_sessi
                parse_request(&reqcpy);
                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);
@@ -2405,7 +2389,8 @@ cleanup:
        ast_free(me);
 cleanup2:
        fclose(ser->f);
-       ser = ast_tcptls_session_instance_destroy(ser);
+       ser->f = NULL;
+       ser->fd = -1;
        if (reqcpy.data)
                ast_free(reqcpy.data);
        if (req.data) {
@@ -2414,11 +2399,8 @@ cleanup2:
        }
        
 
-       if (req.socket.lock) {
-               ast_mutex_destroy(req.socket.lock);
-               ast_free(req.socket.lock);
-               req.socket.lock = NULL;
-       }
+       ao2_ref(ser, -1);
+       ser = NULL;
 
        return NULL;
 }
@@ -2761,8 +2743,8 @@ static int __sip_xmit(struct sip_pvt *p, struct ast_str *data, int len)
        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->str, len, 0, (const struct sockaddr *)dst, sizeof(struct sockaddr_in));
@@ -2773,8 +2755,8 @@ static int __sip_xmit(struct sip_pvt *p, struct ast_str *data, int len)
                        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) {
@@ -3780,6 +3762,11 @@ static void sip_destroy_peer(struct sip_peer *peer)
        if (peer->dnsmgr)
                ast_dnsmgr_release(peer->dnsmgr);
        clear_peer_mailboxes(peer);
+
+       if (peer->socket.ser) {
+               ao2_ref(peer->socket.ser, -1);
+               peer->socket.ser = NULL;
+       }
 }
 
 /*! \brief Update peer data in database (if used) */
@@ -4201,6 +4188,20 @@ static void set_t38_capabilities(struct sip_pvt *p)
        }
 }
 
+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.
@@ -4210,7 +4211,7 @@ static void set_t38_capabilities(struct sip_pvt *p)
  */
 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)))) {
@@ -4652,7 +4653,11 @@ static void __sip_destroy(struct sip_pvt *p, int lockowner, int lockdialoglist)
        }
 
        ast_string_field_free_memory(p);
-       return;
+
+       if (p->socket.ser) {
+               ao2_ref(p->socket.ser, -1);
+               p->socket.ser = NULL;
+       }
 }
 
 /*! \brief  update_call_counter: Handle call_limit for SIP users 
@@ -7946,11 +7951,7 @@ static int transmit_response_using_temp(ast_string_field callid, struct sockaddr
        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);
@@ -10317,7 +10318,8 @@ static enum parse_register_result parse_register_contact(struct sip_pvt *pvt, st
                }
        }
 
-       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;
@@ -19436,7 +19438,6 @@ static int sipsock_read(int *id, int fd, short events, void *ignore)
        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);
        if (req.data) {
@@ -19491,7 +19492,7 @@ static int handle_request_do(struct sip_request *req, struct sockaddr_in *sin)
                        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 */
@@ -19589,13 +19590,18 @@ static int sip_prepare_socket(struct sip_pvt *p)
 
        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)
@@ -19605,7 +19611,12 @@ static int sip_prepare_socket(struct sip_pvt *p)
                                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)
@@ -19615,8 +19626,12 @@ static int sip_prepare_socket(struct sip_pvt *p)
 
        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;
        }

diff --git a/include/asterisk/tcptls.h b/include/asterisk/tcptls.h
index 004a883..a345200 100644 (file)
--- a/include/asterisk/tcptls.h
+++ b/include/asterisk/tcptls.h
@@ -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 @@ struct ast_tcptls_session_instance {
        int client;
        struct sockaddr_in requestor;
        struct server_args *parent;
+       ast_mutex_t lock;
 };
 
 /*! \brief
@@ -166,11 +168,4 @@ void *ast_make_file_from_fd(void *data);
 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 */

diff --git a/main/astobj2.c b/main/astobj2.c
index d8aaca1..e9e2db7 100644 (file)
--- a/main/astobj2.c
+++ b/main/astobj2.c
@@ -930,12 +930,6 @@ static char *handle_astobj2_test(struct ast_cli_entry *e, int cmd, struct ast_cl
                ast_cli(a->fd, "object %d allocated as %p\n", i, obj);
                sprintf(obj, "-- this is obj %d --", i);
                ao2_link(c1, obj);
-               /* At this point, the refcount on obj is 2 due to the allocation
-                * and linking. We can go ahead and reduce the refcount by 1
-                * right here so that when the container is unreffed later, the
-                * objects will be freed
-                */
-               ao2_t_ref(obj, -1, "test");
        }
        ast_cli(a->fd, "testing callbacks\n");
        ao2_t_callback(c1, 0, print_cb, &a->fd,"test callback");

diff --git a/main/http.c b/main/http.c
index 405f65d..33818af 100644 (file)
--- a/main/http.c
+++ b/main/http.c
@@ -736,7 +736,8 @@ static void *httpd_helper_thread(void *data)
 
 done:
        fclose(ser->f);
-       ser = ast_tcptls_session_instance_destroy(ser);
+       ao2_ref(ser, -1);
+       ser = NULL;
 
        return NULL;
 }

diff --git a/main/manager.c b/main/manager.c
index 6af81c6..61d6da5 100644 (file)
--- a/main/manager.c
+++ b/main/manager.c
@@ -3089,7 +3089,8 @@ static void *session_do(void *data)
        destroy_session(s);
 
 done:
-       ser = ast_tcptls_session_instance_destroy(ser);
+       ao2_ref(ser, -1);
+       ser = NULL;
        return NULL;
 }
 

diff --git a/main/tcptls.c b/main/tcptls.c
index 67782a0..9ce3ac9 100644 (file)
--- a/main/tcptls.c
+++ b/main/tcptls.c
@@ -83,6 +83,12 @@ static int ssl_close(void *cookie)
 
 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,6 +98,12 @@ HOOK_T ast_tcptls_server_read(struct ast_tcptls_session_instance *ser, void *buf
 
 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);
@@ -99,6 +111,12 @@ HOOK_T ast_tcptls_server_write(struct ast_tcptls_session_instance *ser, void *bu
        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)
 {
        struct server_args *desc = data;
@@ -123,12 +141,15 @@ void *ast_tcptls_server_root(void *data)
                                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 @@ void *ast_tcptls_server_root(void *data)
                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,9 +256,11 @@ struct ast_tcptls_session_instance *ast_tcptls_client_start(struct server_args *
                goto error;
        }
 
-       if (!(ser = ast_calloc(1, sizeof(*ser))))
+       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 @@ error:
        close(desc->accept_fd);
        desc->accept_fd = -1;
        if (ser)
-               ast_free(ser);
+               ao2_ref(ser, -1);
        return NULL;
 }
 
@@ -447,8 +470,3 @@ void *ast_make_file_from_fd(void *data)
                return ser;
 }
 
-struct ast_tcptls_session_instance *ast_tcptls_session_instance_destroy(struct ast_tcptls_session_instance *i)
-{
-       ast_free(i);
-       return NULL;
-}