Fix problem with closing 0-fds

  - add debugging to registering/unregistering DNS-Watchers
  - sanitize closing of filedescriptors; don't close 0-fd, and set to 0 after close.

diff --git a/citadel/event_client.c b/citadel/event_client.c
index 99f71151e6351b4b16457e125cfc0d8536396781..d9e09b48b12448604c4d88a1ea97efaa0e06251e 100644 (file)
--- a/citadel/event_client.c
+++ b/citadel/event_client.c
@@ -47,6 +47,9 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <assert.h>
+#if HAVE_BACKTRACE
+#include <execinfo.h>
+#endif
 
 #include <libcitadel.h>
 #include "citadel.h"
@@ -256,7 +259,10 @@ void StopClientWatchers(AsyncIO *IO)
        ev_io_stop(event_base, &IO->conn_event);
        ev_io_stop(event_base, &IO->send_event);
        ev_io_stop(event_base, &IO->recv_event);
-       close(IO->SendBuf.fd);
+
+       if (IO->SendBuf.fd != 0) {
+               close(IO->SendBuf.fd);
+       }
        IO->SendBuf.fd = 0;
        IO->RecvBuf.fd = 0;
 }
@@ -273,6 +279,8 @@ void ShutDownCLient(AsyncIO *IO)
 
        if (IO->DNS.Channel != NULL) {
                ares_destroy(IO->DNS.Channel);
+               EV_DNS_LOG_STOP(DNS.recv_event);
+               EV_DNS_LOG_STOP(DNS.send_event);
                ev_io_stop(event_base, &IO->DNS.recv_event);
                ev_io_stop(event_base, &IO->DNS.send_event);
                IO->DNS.Channel = NULL;
@@ -707,6 +715,7 @@ IO_postdns_callback(struct ev_loop *loop, ev_idle *watcher, int revents)
        case eAbort:
                switch (IO->DNS.Fail(IO)) {
                case eAbort:
+////                   StopClientWatchers(IO);
                        ShutDownCLient(IO);
                default:
                        break;
@@ -748,6 +757,7 @@ eNextState EvConnectSock(AsyncIO *IO,
                StrBufPrintf(IO->ErrMsg,
                             "Failed to create socket: %s",
                             strerror(errno));
+               IO->SendBuf.fd = IO->RecvBuf.fd = 0;
                return eAbort;
        }
        fdflags = fcntl(IO->SendBuf.fd, F_GETFL);
@@ -758,6 +768,8 @@ eNextState EvConnectSock(AsyncIO *IO,
                StrBufPrintf(IO->ErrMsg,
                             "Failed to get socket flags: %s",
                             strerror(errno));
+               close(IO->SendBuf.fd);
+               IO->SendBuf.fd = IO->RecvBuf.fd = 0;
                return eAbort;
        }
        fdflags = fdflags | O_NONBLOCK;
@@ -770,7 +782,7 @@ eNextState EvConnectSock(AsyncIO *IO,
                             "Failed to set socket flags: %s",
                             strerror(errno));
                close(IO->SendBuf.fd);
-               IO->SendBuf.fd = IO->RecvBuf.fd = -1;
+               IO->SendBuf.fd = IO->RecvBuf.fd = 0;
                return eAbort;
        }
 /* TODO: maye we could use offsetof() to calc the position of data...
@@ -893,6 +905,9 @@ void InitIOStruct(AsyncIO *IO,
        IO->SendBuf.Buf   = NewStrBufPlain(NULL, 1024);
        IO->RecvBuf.Buf   = NewStrBufPlain(NULL, 1024);
        IO->IOBuf         = NewStrBuf();
+       EV_syslog(LOG_DEBUG,
+                 "EVENT: Session lives at %p IO at %p \n",
+                 Data, IO);
 
 }
 
@@ -920,3 +935,23 @@ int InitcURLIOStruct(AsyncIO *IO,
        return  evcurl_init(IO);
 
 }
+
+void EV_backtrace(AsyncIO *IO)
+{
+#ifdef HAVE_BACKTRACE
+       void *stack_frames[50];
+       size_t size, i;
+       char **strings;
+
+
+       size = backtrace(stack_frames, sizeof(stack_frames) / sizeof(void*));
+       strings = backtrace_symbols(stack_frames, size);
+       for (i = 0; i < size; i++) {
+               if (strings != NULL)
+                       EV_syslog(LOG_ALERT, " BT %s\n", strings[i]);
+               else
+                       EV_syslog(LOG_ALERT, " BT %p\n", stack_frames[i]);
+       }
+       free(strings);
+#endif
+}

diff --git a/citadel/event_client.h b/citadel/event_client.h
index 8fcef9fdecbaf53ded1f02a860a5525cc2c0b5f9..66f7f42e1782d0deb42594e8f9687d3db73e7594 100644 (file)
--- a/citadel/event_client.h
+++ b/citadel/event_client.h
@@ -187,6 +187,39 @@ typedef struct _IOAddHandler {
 
 #define EVNCM_syslog(LEVEL, FORMAT) syslog(LEVEL, "IO[%ld]" FORMAT, IO->ID)
 
+#ifdef DEBUG_CARES
+#define EV_DNS_LOG_START(a)                                                    \
+       syslog(LOG_DEBUG, "IO[%ld]CC[%d] + Starting " #a " %p FD %d", IO->ID, CCID, &IO->a, IO->a.fd); \
+       EV_backtrace(IO);
+
+#define EV_DNS_LOG_STOP(a)                                                     \
+       syslog(LOG_DEBUG, "IO[%ld]CC[%d] - Stopping " #a " %p FD %d", IO->ID, CCID, &IO->a, IO->a.fd); \
+       EV_backtrace(IO);
+
+#define EV_DNS_LOG_INIT(a)                                                     \
+       syslog(LOG_DEBUG, "IO[%ld]CC[%d] * Init " #a " %p FD %d", IO->ID, CCID, &IO->a, IO->a.fd); \
+       EV_backtrace(IO);
+
+#define EV_DNS_LOGT_START(a)                                                   \
+       syslog(LOG_DEBUG, "IO[%ld]CC[%d] + Starting " #a " %p", IO->ID, CCID, &IO->a); \
+       EV_backtrace(IO);
+
+#define EV_DNS_LOGT_STOP(a)                                                    \
+       syslog(LOG_DEBUG, "IO[%ld]CC[%d] - Stopping " #a " %p", IO->ID, CCID, &IO->a); \
+       EV_backtrace(IO);
+
+#define EV_DNS_LOGT_INIT(a)                                                    \
+       syslog(LOG_DEBUG, "IO[%ld]CC[%d] * Init " #a " %p", IO->ID, CCID, &IO->a); \
+       EV_backtrace(IO);
+#else
+#define EV_DNS_LOG_START(a)
+#define EV_DNS_LOG_STOP(a)
+#define EV_DNS_LOG_INIT(a)
+#define EV_DNS_LOGT_START(a)
+#define EV_DNS_LOGT_STOP(a)
+#define EV_DNS_LOGT_INIT(a)
+#endif
+
 void FreeAsyncIOContents(AsyncIO *IO);
 
 eNextState NextDBOperation(AsyncIO *IO, IO_CallBack CB);
@@ -256,4 +289,6 @@ eNextState ReAttachIO(AsyncIO *IO,
                      void *pData,
                      int ReadFirst);
 
+void EV_backtrace(AsyncIO *IO);
+
 #endif /* __EVENT_CLIENT_H__ */

diff --git a/citadel/modules/c-ares-dns/serv_c-ares-dns.c b/citadel/modules/c-ares-dns/serv_c-ares-dns.c
index a22d6c55794a3ce590c75b1bb396ec5a4cddde9a..e945ccf9f785da5f30de87acbc23942e8a31cd49 100644 (file)
--- a/citadel/modules/c-ares-dns/serv_c-ares-dns.c
+++ b/citadel/modules/c-ares-dns/serv_c-ares-dns.c
@@ -71,6 +71,7 @@ static void HostByAddrCb(void *data,
        AsyncIO *IO = data;
 #ifdef DEBUG_CARES
        EV_syslog(LOG_DEBUG, "C-ARES: %s\n", __FUNCTION__);
+       EV_DNS_LOGT_STOP(DNS.timeout);
 #endif
        ev_timer_stop (event_base, &IO->DNS.timeout);
 
@@ -261,6 +262,7 @@ void QueryCb(void *arg,
        AsyncIO *IO = arg;
 #ifdef DEBUG_CARES
        EV_syslog(LOG_DEBUG, "C-ARES: %s\n", __FUNCTION__);
+       EV_DNS_LOGT_STOP(DNS.timeout);
 #endif
        ev_timer_stop (event_base, &IO->DNS.timeout);
 
@@ -278,6 +280,8 @@ void QueryCb(void *arg,
        ev_idle_init(&IO->unwind_stack,
                     IO_postdns_callback);
        IO->unwind_stack.data = IO;
+       EV_DNS_LOGT_INIT(unwind_stack);
+       EV_DNS_LOGT_START(unwind_stack);
        ev_idle_start(event_base, &IO->unwind_stack);
 }
 
@@ -285,6 +289,7 @@ void QueryCbDone(AsyncIO *IO)
 {
 #ifdef DEBUG_CARES
        EV_syslog(LOG_DEBUG, "C-ARES: %s\n", __FUNCTION__);
+       EV_DNS_LOGT_STOP(DNS.timeout);
 #endif
 
        ev_idle_stop(event_base, &IO->unwind_stack);
@@ -355,6 +360,8 @@ void QueueGetHostByNameDone(void *Ctx,
        ev_idle_init(&IO->unwind_stack,
                     IO_postdns_callback);
        IO->unwind_stack.data = IO;
+       EV_DNS_LOGT_INIT(unwind_stack);
+       EV_DNS_LOGT_START(unwind_stack);
        ev_idle_start(event_base, &IO->unwind_stack);
 }
 
@@ -374,12 +381,14 @@ void QueueGetHostByName(AsyncIO *IO,
        InitC_ares_dns(IO);
 
        ev_timer_init(&IO->DNS.timeout, DNStimeouttrigger_callback, 10, 1);
+       EV_DNS_LOGT_INIT(DNS.timeout);
        IO->DNS.timeout.data = IO;
        ares_gethostbyname(IO->DNS.Channel,
                           Hostname,
                           AF_INET6, /* it falls back to ipv4 in doubt... */
                           QueueGetHostByNameDone,
                           IO);
+       EV_DNS_LOGT_START(DNS.timeout);
        ev_timer_start(event_base, &IO->DNS.timeout);
 
 }
@@ -404,6 +413,7 @@ int QueueQuery(ns_type Type,
 
        ev_timer_init(&IO->DNS.timeout, DNStimeouttrigger_callback, 10, 1);
        IO->DNS.timeout.data = IO;
+       EV_DNS_LOGT_INIT(DNS.timeout);
 
        switch(Type) {
        case ns_t_a:
@@ -453,6 +463,7 @@ int QueueQuery(ns_type Type,
                                   family,
                                   HostByAddrCb,
                                   IO);
+               EV_DNS_LOGT_START(DNS.timeout);
                ev_timer_start(event_base, &IO->DNS.timeout);
 #ifdef DEBUG_CARES
                EV_syslog(LOG_DEBUG, "C-ARES: %s X1\n", __FUNCTION__);
@@ -469,6 +480,7 @@ int QueueQuery(ns_type Type,
        EV_syslog(LOG_DEBUG, "C-ARES: %s\n", __FUNCTION__);
 #endif
        ares_query(IO->DNS.Channel, name, ns_c_in, Type, QueryCb, IO);
+       EV_DNS_LOGT_START(DNS.timeout);
        ev_timer_start(event_base, &IO->DNS.timeout);
        return 1;
 }
@@ -531,6 +543,7 @@ void SockStateCb(void *data, int sock, int read, int write)
        if (read) {
                if ((IO->DNS.recv_event.fd != sock) &&
                    (IO->DNS.recv_event.fd != 0)) {
+                       EV_DNS_LOG_STOP(DNS.recv_event);
                        ev_io_stop(event_base, &IO->DNS.recv_event);
                }
                IO->DNS.recv_event.fd = sock;
@@ -538,12 +551,15 @@ void SockStateCb(void *data, int sock, int read, int write)
                           DNS_recv_callback,
                           IO->DNS.recv_event.fd,
                           EV_READ);
+               EV_DNS_LOG_INIT(DNS.recv_event);
                IO->DNS.recv_event.data = IO;
+               EV_DNS_LOG_START(DNS.recv_event);
                ev_io_start(event_base, &IO->DNS.recv_event);
        }
        if (write) {
                if ((IO->DNS.send_event.fd != sock) &&
                    (IO->DNS.send_event.fd != 0)) {
+                       EV_DNS_LOG_STOP(DNS.send_event);
                        ev_io_stop(event_base, &IO->DNS.send_event);
                }
                IO->DNS.send_event.fd = sock;
@@ -552,9 +568,13 @@ void SockStateCb(void *data, int sock, int read, int write)
                           IO->DNS.send_event.fd,
                           EV_WRITE);
                IO->DNS.send_event.data = IO;
+               EV_DNS_LOG_INIT(DNS.send_event);
+               EV_DNS_LOG_START(DNS.send_event);
                ev_io_start(event_base, &IO->DNS.send_event);
        }
        if ((read == 0) && (write == 0)) {
+               EV_DNS_LOG_STOP(DNS.recv_event);
+               EV_DNS_LOG_STOP(DNS.send_event);
                ev_io_stop(event_base, &IO->DNS.recv_event);
                ev_io_stop(event_base, &IO->DNS.send_event);
        }

diff --git a/citadel/modules/smtp/serv_smtpeventclient.c b/citadel/modules/smtp/serv_smtpeventclient.c
index 43be90fdc61c36d04d5a95afb00887d2f62e37c8..b38320d2a64b1923c0d4a92103b9cc93913c3178 100644 (file)
--- a/citadel/modules/smtp/serv_smtpeventclient.c
+++ b/citadel/modules/smtp/serv_smtpeventclient.c
@@ -94,6 +94,8 @@ const unsigned short DefaultMXPort = 25;
 void DeleteSmtpOutMsg(void *v)
 {
        SmtpOutMsg *Msg = v;
+       AsyncIO *IO = &Msg->IO;
+       EV_syslog(LOG_DEBUG, "SMTP: %s Aborting\n", __FUNCTION__);
 
        /* these are kept in our own space and free'd below */
        Msg->IO.ConnectMe = NULL;
@@ -104,7 +106,7 @@ void DeleteSmtpOutMsg(void *v)
        FreeURL(&Msg->Relay);
        FreeStrBuf(&Msg->msgtext);
        FreeAsyncIOContents(&Msg->IO);
-       memset (Msg, 0, sizeof(SmtpOutMsg)); /* just to be shure... */
+///    memset (Msg, 0, sizeof(SmtpOutMsg)); /* just to be shure... */
        free(Msg);
 }
 
@@ -330,6 +332,8 @@ eNextState get_one_mx_host_ip(AsyncIO *IO)
         * - one of the mx'es
         */
 
+       InitC_ares_dns(IO);
+
        EVS_syslog(LOG_DEBUG, "SMTP: %s\n", __FUNCTION__);
 
        EVS_syslog(LOG_DEBUG,

diff --git a/citadel/sysdep.c b/citadel/sysdep.c
index 816ed3f16d0f527627d6571c4746d7406bef580d..8f71b7ea41ec3c4a09882d727ed3454ca417f308 100644 (file)
--- a/citadel/sysdep.c
+++ b/citadel/sysdep.c
@@ -927,16 +927,18 @@ void close_masters (void)
 
                if (serviceptr->tcp_port > 0)
                {
-                       syslog(LOG_INFO, "Closing listener on port %d\n",
-                               serviceptr->tcp_port);
+                       syslog(LOG_INFO, "Closing %d listener on port %d\n",
+                              serviceptr->msock,
+                              serviceptr->tcp_port);
                        serviceptr->tcp_port = 0;
                }
                
                if (serviceptr->sockpath != NULL)
-                       syslog(LOG_INFO, "Closing listener on '%s'\n",
-                               serviceptr->sockpath);
-
-               close(serviceptr->msock);
+                       syslog(LOG_INFO, "Closing %d listener on '%s'\n",
+                              serviceptr->msock,
+                              serviceptr->sockpath);
+                if (serviceptr->msock != -1)
+                       close(serviceptr->msock);
                /* If it's a Unix domain socket, remove the file. */
                if (serviceptr->sockpath != NULL) {
                        unlink(serviceptr->sockpath);