Add support for a "debug mode" to try to catch common errors.

Right now it only catches cases where we aren't initializing events,
or where we are re-initializing events without deleting them first.
These are however shockingly common.

diff --git a/configure.in b/configure.in
index 87c625c040255fb9426d052876bbf279db82e6ba..bcca1b90fca697ccd5d0e08bd7a8750efba301f9 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -31,7 +31,7 @@ fi
 AC_ARG_ENABLE(gcc-warnings,
      AS_HELP_STRING(--enable-gcc-warnings, enable verbose warnings with GCC))
 AC_ARG_ENABLE(thread-support,
-     AS_HELP_STRING(--enable-thread-support, enable support for threading),
+     AS_HELP_STRING(--disable-thread-support, disable support for threading),
        [], [enable_thread_support=yes])
 AC_ARG_ENABLE(malloc-replacement,
      AS_HELP_STRING(--disable-malloc-replacement, disable support for replacing the memory mgt functions),
@@ -39,6 +39,9 @@ AC_ARG_ENABLE(malloc-replacement,
 AC_ARG_ENABLE(openssl,
      AS_HELP_STRING(--disable-openssl, disable support for openssl encryption),
         [], [enable_openssl=yes])
+AC_ARG_ENABLE(debug-mode,
+     AS_HELP_STRING(--disable-debug-mode, disable support for running in debug mode),
+        [], [enable_debug_mode=yes])
 
 AC_PROG_LIBTOOL
 
@@ -529,6 +532,12 @@ if test x$enable_malloc_replacement = xno; then
         [Define if libevent should not allow replacing the mm functions])
 fi
 
+# check if we should hard-code debugging out
+if test x$enable_debug_mode = xno; then
+  AC_DEFINE(DISABLE_DEBUG_MODE, 1,
+        [Define if libevent should build without support for a debug mode])
+fi
+
 # check if we have and should use openssl
 AM_CONDITIONAL(OPENSSL, [test "$enable_openssl" != "no" && test "$have_openssl" = "yes"])
 

diff --git a/event-internal.h b/event-internal.h
index fe6027fa0b3f3c4d5798db5b3293fceb701303ef..9bba68e6e6ca5d62ef1d596d1e578b566a1656b9 100644 (file)
--- a/event-internal.h
+++ b/event-internal.h
@@ -122,6 +122,10 @@ struct event_changelist {
        int changes_size;
 };
 
+#ifndef _EVENT_DISABLE_DEBUG_MODE
+extern int _event_debug_mode_on;
+#endif
+
 struct event_base {
        /** Function pointers and other data to describe this event_base's
         * backend. */

diff --git a/event.c b/event.c
index 722e93669def051500220aca818ce9eea3c9e069..d12b72e758546dbb205dcb82448a3b55701492b7 100644 (file)
--- a/event.c
+++ b/event.c
@@ -66,6 +66,7 @@
 #include "evmap-internal.h"
 #include "iocp-internal.h"
 #include "changelist-internal.h"
+#include "ht-internal.h"
 
 #ifdef _EVENT_HAVE_EVENT_PORTS
 extern const struct eventop evportops;
@@ -140,6 +141,150 @@ static inline void        event_persist_closure(struct event_base *, struct event *ev);
 
 static int     evthread_notify_base(struct event_base *base);
 
+#ifndef _EVENT_DISABLE_DEBUG_MODE
+/* These functions implement a hashtable of which 'struct event *' structures
+ * have been setup or added.  We don't want to trust the content of the struct
+ * event itself, since we're trying to work through cases where an event gets
+ * clobbered or freed.  Instead, we keep a hashtable indexed by the pointer.
+ */
+
+struct event_debug_entry {
+       HT_ENTRY(event_debug_entry) node;
+       const struct event *ptr;
+       unsigned added : 1;
+};
+
+static inline unsigned
+hash_debug_entry(const struct event_debug_entry *e)
+{
+       return ((unsigned)e->ptr) >> 3;
+}
+
+static inline int
+eq_debug_entry(const struct event_debug_entry *a,
+    const struct event_debug_entry *b)
+{
+       return a->ptr == b->ptr;
+}
+
+int _event_debug_mode_on = 0;
+static void *_event_debug_map_lock = NULL;
+static HT_HEAD(event_debug_map, event_debug_entry) global_debug_map =
+       HT_INITIALIZER();
+
+HT_PROTOTYPE(event_debug_map, event_debug_entry, node, hash_debug_entry,
+    eq_debug_entry);
+HT_GENERATE(event_debug_map, event_debug_entry, node, hash_debug_entry,
+    eq_debug_entry, 0.5, mm_malloc, mm_realloc, mm_free);
+
+#define _event_debug_note_setup(ev) do {                               \
+       if (_event_debug_mode_on) {                                     \
+               struct event_debug_entry *dent,find;                    \
+               find.ptr = (ev);                                        \
+               EVLOCK_LOCK(_event_debug_map_lock, 0);                  \
+               dent = HT_FIND(event_debug_map, &global_debug_map, &find); \
+               if (dent) {                                             \
+                       dent->added = 0;                                \
+               } else {                                                \
+                       dent = mm_malloc(sizeof(*dent));                \
+                       if (!dent)                                      \
+                               event_err(1,                            \
+                                   "Out of memory in debugging code"); \
+                       dent->ptr = (ev);                               \
+                       dent->added = 0;                                \
+                       HT_INSERT(event_debug_map, &global_debug_map, dent); \
+               }                                                       \
+               EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
+       }                                                               \
+       } while (0)
+#define _event_debug_note_teardown(ev) do {                            \
+       if (_event_debug_mode_on) {                                     \
+               struct event_debug_entry *dent,find;                    \
+               find.ptr = (ev);                                        \
+               EVLOCK_LOCK(_event_debug_map_lock, 0);                  \
+               dent = HT_REMOVE(event_debug_map, &global_debug_map, &find); \
+               if (dent)                                               \
+                       mm_free(dent);                                  \
+               EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
+       }                                                               \
+       } while(0)
+#define _event_debug_note_add(ev)      do {                            \
+       if (_event_debug_mode_on) {                                     \
+               struct event_debug_entry *dent,find;                    \
+               find.ptr = (ev);                                        \
+               EVLOCK_LOCK(_event_debug_map_lock, 0);                  \
+               dent = HT_FIND(event_debug_map, &global_debug_map, &find); \
+               if (dent) {                                             \
+                       dent->added = 1;                                \
+               } else {                                                \
+                       event_errx(_EVENT_ERR_ABORT,                    \
+                           "%s: noting an add on a non-setup event %p", \
+                           __func__, (ev));                            \
+               }                                                       \
+               EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
+       }                                                               \
+       } while(0)
+#define _event_debug_note_del(ev) do {                                 \
+       if (_event_debug_mode_on) {                                     \
+               struct event_debug_entry *dent,find;                    \
+               find.ptr = (ev);                                        \
+               EVLOCK_LOCK(_event_debug_map_lock, 0);                  \
+               dent = HT_FIND(event_debug_map, &global_debug_map, &find); \
+               if (dent) {                                             \
+                       dent->added = 0;                                \
+               } else {                                                \
+                       event_errx(_EVENT_ERR_ABORT,                    \
+                           "%s: noting a del on a non-setup event %p", \
+                           __func__, (ev));                            \
+               }                                                       \
+               EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
+       }                                                               \
+       } while(0)
+#define _event_debug_assert_is_setup(ev) do {                          \
+       if (_event_debug_mode_on) {                                     \
+               struct event_debug_entry *dent,find;                    \
+               find.ptr = (ev);                                        \
+               EVLOCK_LOCK(_event_debug_map_lock, 0);                  \
+               dent = HT_FIND(event_debug_map, &global_debug_map, &find); \
+               if (!dent) {                                            \
+                       event_errx(_EVENT_ERR_ABORT,                    \
+                           "%s called on a non-initialized event %p",  \
+                           __func__, (ev));                            \
+               }                                                       \
+               EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
+       }                                                               \
+       } while (0)
+
+#define _event_debug_assert_not_added(ev) do {                         \
+       if (_event_debug_mode_on) {                                     \
+               struct event_debug_entry *dent,find;                    \
+               find.ptr = (ev);                                        \
+               EVLOCK_LOCK(_event_debug_map_lock, 0);                  \
+               dent = HT_FIND(event_debug_map, &global_debug_map, &find); \
+               if (dent && dent->added) {                              \
+                       event_errx(_EVENT_ERR_ABORT,                    \
+                           "%s called on an already added event %p",   \
+                           __func__, (ev));                            \
+               }                                                       \
+               EVLOCK_UNLOCK(_event_debug_map_lock, 0);                \
+       }                                                               \
+       } while (0)
+
+#else
+#define _event_debug_note_setup(ev) \
+       ((void)0)
+#define _event_debug_note_teardown(ev) \
+       ((void)0)
+#define _event_debug_note_add(ev) \
+       ((void)0)
+#define _event_debug_note_del(ev) \
+       ((void)0)
+#define _event_debug_assert_is_setup(ev) \
+       ((void)0)
+#define _event_debug_assert_not_added(ev) \
+       ((void)0)
+#endif
+
 static void
 detect_monotonic(void)
 {
@@ -291,6 +436,38 @@ event_base_get_deferred_cb_queue(struct event_base *base)
        return base ? &base->defer_queue : NULL;
 }
 
+void
+event_enable_debug_mode(void)
+{
+#ifndef _EVENT_DISABLE_DEBUG_MODE
+       if (_event_debug_mode_on)
+               event_errx(1, "%s was called twice!", __func__);
+
+       _event_debug_mode_on = 1;
+
+       HT_INIT(event_debug_map, &global_debug_map);
+
+       EVTHREAD_ALLOC_LOCK(_event_debug_map_lock, 0);
+#endif
+}
+
+#if 0
+void
+event_disable_debug_mode(void)
+{
+       struct event_debug_entry **ent, *victim;
+
+       EVLOCK_LOCK(_event_debug_map_lock, 0);
+       for (ent = HT_START(event_debug_map, &global_debug_map); ent; ) {
+               victim = *ent;
+               ent = HT_NEXT_RMV(event_debug_map,&global_debug_map, ent);
+               mm_free(victim);
+       }
+       HT_CLEAR(event_debug_map, &global_debug_map);
+       EVLOCK_UNLOCK(_event_debug_map_lock , 0);
+}
+#endif
+
 struct event_base *
 event_base_new_with_config(struct event_config *cfg)
 {
@@ -298,6 +475,10 @@ event_base_new_with_config(struct event_config *cfg)
        struct event_base *base;
        int should_check_environment;
 
+       if (_event_debug_mode_on && !_event_debug_map_lock) {
+               EVTHREAD_ALLOC_LOCK(_event_debug_map_lock, 0);
+       }
+
        if ((base = mm_calloc(1, sizeof(struct event_base))) == NULL) {
                event_warn("%s: calloc", __func__);
                return NULL;
@@ -619,7 +800,6 @@ event_config_free(struct event_config *cfg)
        mm_free(cfg);
 }
 
-
 int
 event_config_set_flag(struct event_config *cfg, int flag)
 {
@@ -1282,6 +1462,9 @@ event_assign(struct event *ev, struct event_base *base, evutil_socket_t fd, shor
 {
        if (!base)
                base = current_base;
+
+       _event_debug_assert_not_added(ev);
+
        ev->ev_base = base;
 
        ev->ev_callback = callback;
@@ -1315,6 +1498,9 @@ event_assign(struct event *ev, struct event_base *base, evutil_socket_t fd, shor
                /* by default, we put new events into the middle priority */
                ev->ev_pri = base->nactivequeues / 2;
        }
+
+       _event_debug_note_setup(ev);
+
        return 0;
 }
 
@@ -1325,6 +1511,8 @@ event_base_set(struct event_base *base, struct event *ev)
        if (ev->ev_flags != EVLIST_INIT)
                return (-1);
 
+       _event_debug_assert_is_setup(ev);
+
        ev->ev_base = base;
        ev->ev_pri = base->nactivequeues/2;
 
@@ -1358,9 +1546,22 @@ event_new(struct event_base *base, evutil_socket_t fd, short events, void (*cb)(
 void
 event_free(struct event *ev)
 {
+       _event_debug_assert_is_setup(ev);
+
        /* make sure that this event won't be coming back to haunt us. */
        event_del(ev);
+       _event_debug_note_teardown(ev);
        mm_free(ev);
+
+}
+
+void
+event_debug_unassign(struct event *ev)
+{
+       _event_debug_assert_not_added(ev);
+       _event_debug_note_teardown(ev);
+
+       ev->ev_flags &= ~EVLIST_INIT;
 }
 
 /*
@@ -1371,6 +1572,8 @@ event_free(struct event *ev)
 int
 event_priority_set(struct event *ev, int pri)
 {
+       _event_debug_assert_is_setup(ev);
+
        if (ev->ev_flags & EVLIST_ACTIVE)
                return (-1);
        if (pri < 0 || pri >= ev->ev_base->nactivequeues)
@@ -1391,6 +1594,8 @@ event_pending(struct event *ev, short event, struct timeval *tv)
        struct timeval  now, res;
        int flags = 0;
 
+       _event_debug_assert_is_setup(ev);
+
        if (ev->ev_flags & EVLIST_INSERTED)
                flags |= (ev->ev_events & (EV_READ|EV_WRITE|EV_SIGNAL));
        if (ev->ev_flags & EVLIST_ACTIVE)
@@ -1430,6 +1635,8 @@ _event_initialized(struct event *ev, int need_fd)
 void
 event_get_assignment(const struct event *event, struct event_base **base_out, evutil_socket_t *fd_out, short *events_out, event_callback_fn *callback_out, void **arg_out)
 {
+       _event_debug_assert_is_setup(event);
+
        if (base_out)
                *base_out = event->ev_base;
        if (fd_out)
@@ -1451,30 +1658,35 @@ event_get_struct_event_size(void)
 evutil_socket_t
 event_get_fd(const struct event *ev)
 {
+       _event_debug_assert_is_setup(ev);
        return ev->ev_fd;
 }
 
 struct event_base *
 event_get_base(const struct event *ev)
 {
+       _event_debug_assert_is_setup(ev);
        return ev->ev_base;
 }
 
 short
 event_get_events(const struct event *ev)
 {
+       _event_debug_assert_is_setup(ev);
        return ev->ev_events;
 }
 
 event_callback_fn
 event_get_callback(const struct event *ev)
 {
+       _event_debug_assert_is_setup(ev);
        return ev->ev_callback;
 }
 
 void *
 event_get_callback_arg(const struct event *ev)
 {
+       _event_debug_assert_is_setup(ev);
        return ev->ev_arg;
 }
 
@@ -1536,6 +1748,8 @@ event_add_internal(struct event *ev, const struct timeval *tv,
        int res = 0;
        int notify = 0;
 
+       _event_debug_assert_is_setup(ev);
+
        event_debug((
                 "event_add: event: %p, %s%s%scall %p",
                 ev,
@@ -1619,7 +1833,6 @@ event_add_internal(struct event *ev, const struct timeval *tv,
 
                gettime(base, &now);
 
-
                common_timeout = is_common_timeout(tv, base);
                if (tv_is_absolute) {
                        ev->ev_timeout = *tv;
@@ -1658,6 +1871,8 @@ event_add_internal(struct event *ev, const struct timeval *tv,
        if (res != -1 && notify && !EVBASE_IN_THREAD(base))
                evthread_notify_base(base);
 
+       _event_debug_note_add(ev);
+
        return (res);
 }
 
@@ -1744,6 +1959,8 @@ event_del_internal(struct event *ev)
        if (need_cur_lock)
                EVBASE_RELEASE_LOCK(base, current_event_lock);
 
+       _event_debug_note_del(ev);
+
        return (res);
 }
 
@@ -1752,6 +1969,8 @@ event_active(struct event *ev, int res, short ncalls)
 {
        EVBASE_ACQUIRE_LOCK(ev->ev_base, th_base_lock);
 
+       _event_debug_assert_is_setup(ev);
+
        event_active_nolock(ev, res, ncalls);
 
        EVBASE_RELEASE_LOCK(ev->ev_base, th_base_lock);

diff --git a/include/event2/event.h b/include/event2/event.h
index 6ee2543a63bb4923f402f22f35ddcad3e9e01e21..a643264bd18fe8068eca1947b6dd1c5993ec28c1 100644 (file)
--- a/include/event2/event.h
+++ b/include/event2/event.h
@@ -60,6 +60,34 @@ struct event_base;
 struct event;
 struct event_config;
 
+/** Enable some relatively expensive debugging checks in Libevent that would
+ * normally be turned off.  Generally, these cause code that would otherwise
+ * crash mysteriously to fail earlier with an assertion failure.  Note that
+ * this method MUST be called before any events or event_bases have been
+ * created.
+ *
+ * Debug mode can currently catch the following errors:
+ *    An event is re-assigned while it is added
+ *    Any function is called on a non-assigned event
+ *
+ * Note that debugging mode uses memory to track every event that has been
+ * initialized (via event_assign, event_set, or event_new) but not yet
+ * released (via event_free or event_debug_unassign).  If you want to use
+ * debug mode, and you find yourself running out of memory, you will need
+ * to use event_debug_unassign to explicitly stop tracking events that
+ * are no longer considered set-up.
+ */
+void event_enable_debug_mode(void);
+
+/**
+ * When debugging mode is enabled, informs Libevent that an event should no
+ * longer be considered as assigned. When debugging mode is not enabled, does
+ * nothing.
+ *
+ * This function must only be called on a non-added event.
+ */
+void event_debug_unassign(struct event *);
+
 /**
   Initialize the event API.
 
@@ -171,7 +199,7 @@ enum event_base_config_flag {
        /** Instead of checking the current time every time the event loop is
            ready to run timeout callbacks, check after each timeout callback.
         */
-       EVENT_BASE_FLAG_NO_CACHE_TIME = 0x08
+       EVENT_BASE_FLAG_NO_CACHE_TIME = 0x08,
 };
 
 /**
@@ -202,8 +230,8 @@ int event_base_get_features(struct event_base *base);
 */
 int event_config_require_features(struct event_config *cfg, int feature);
 
-/** Sets a flag to configure what parts of the eventual event_base will
- * be initialized, and how they'll work. */
+/** Sets one or more flags to configure what parts of the eventual event_base
+ * will be initialized, and how they'll work. */
 int event_config_set_flag(struct event_config *cfg, int flag);
 
 /**