lib: Try fixing ec6e672a6e32 (ioloop timeout fixing)

Previous code was broken at least when moving a timeout between ioloops.
This is now tested and works.

diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am
index 901a56f626af640c13706a613fbcf5da64e2944b..c13b4ce7a3028471a3cebffa29058b3a144948a6 100644 (file)
--- a/src/lib/Makefile.am
+++ b/src/lib/Makefile.am
@@ -304,6 +304,7 @@ test_lib_SOURCES = \
        test-hash-format.c \
        test-hash-method.c \
        test-hex-binary.c \
+       test-ioloop.c \
        test-iso8601-date.c \
        test-istream.c \
        test-istream-base64-decoder.c \

diff --git a/src/lib/ioloop-private.h b/src/lib/ioloop-private.h
index b149f0c470277cdd191177ffdb575036b6d90239..9fbb18c25297a60791260fa73b9df75ad2879575 100644 (file)
--- a/src/lib/ioloop-private.h
+++ b/src/lib/ioloop-private.h
@@ -3,6 +3,7 @@
 
 #include "priorityq.h"
 #include "ioloop.h"
+#include "array-decl.h"
 
 #ifndef IOLOOP_INITIAL_FD_COUNT
 #  define IOLOOP_INITIAL_FD_COUNT 128
@@ -16,6 +17,7 @@ struct ioloop {
        struct io_file *io_files;
        struct io_file *next_io_file;
        struct priorityq *timeouts;
+       ARRAY(struct timeout *) timeouts_new;
 
         struct ioloop_handler_context *handler_context;
         struct ioloop_notify_handler_context *notify_handler_context;

diff --git a/src/lib/ioloop.c b/src/lib/ioloop.c
index 70d064f827d9ccd690eab4772c51944848bed4f8..87e452491a697fc8fa34ac36878ad7827ad28f6e 100644 (file)
--- a/src/lib/ioloop.c
+++ b/src/lib/ioloop.c
@@ -204,7 +204,8 @@ timeout_add_common(unsigned int source_linenum,
        struct timeout *timeout;
 
        timeout = i_new(struct timeout, 1);
-        timeout->source_linenum = source_linenum;
+       timeout->item.idx = UINT_MAX;
+       timeout->source_linenum = source_linenum;
        timeout->ioloop = current_ioloop;
 
        timeout->callback = callback;
@@ -227,9 +228,15 @@ struct timeout *timeout_add(unsigned int msecs, unsigned int source_linenum,
        timeout = timeout_add_common(source_linenum, callback, context);
        timeout->msecs = msecs;
 
-       timeout_update_next(timeout, timeout->ioloop->running ?
+       if (msecs > 0) {
+               /* start this timeout in the next run cycle */
+               array_append(&timeout->ioloop->timeouts_new, &timeout, 1);
+       } else {
+               /* trigger zero timeouts as soon as possible */
+               timeout_update_next(timeout, timeout->ioloop->running ?
                            NULL : &ioloop_timeval);
-       priorityq_add(timeout->ioloop->timeouts, &timeout->item);
+               priorityq_add(timeout->ioloop->timeouts, &timeout->item);
+       }
        return timeout;
 }
 
@@ -267,7 +274,13 @@ timeout_copy(const struct timeout *old_to)
        new_to->one_shot = old_to->one_shot;
        new_to->msecs = old_to->msecs;
        new_to->next_run = old_to->next_run;
-       priorityq_add(new_to->ioloop->timeouts, &new_to->item);
+
+       if (old_to->item.idx != UINT_MAX)
+               priorityq_add(new_to->ioloop->timeouts, &new_to->item);
+       else if (!new_to->one_shot) {
+               i_assert(new_to->msecs > 0);
+               array_append(&new_to->ioloop->timeouts_new, &new_to, 1);
+       }
 
        return new_to;
 }
@@ -282,16 +295,30 @@ static void timeout_free(struct timeout *timeout)
 void timeout_remove(struct timeout **_timeout)
 {
        struct timeout *timeout = *_timeout;
+       struct ioloop *ioloop = timeout->ioloop;
 
        *_timeout = NULL;
        if (timeout->item.idx != UINT_MAX)
                priorityq_remove(timeout->ioloop->timeouts, &timeout->item);
+       else if (!timeout->one_shot && timeout->msecs > 0) {
+               struct timeout *const *to_idx;
+               array_foreach(&ioloop->timeouts_new, to_idx) {
+                       if (*to_idx == timeout) {
+                               array_delete(&ioloop->timeouts_new,
+                                       array_foreach_idx(&ioloop->timeouts_new, to_idx), 1);
+                               break;
+                       }
+               }
+       }
        timeout_free(timeout);
 }
 
 static void ATTR_NULL(2)
 timeout_reset_timeval(struct timeout *timeout, struct timeval *tv_now)
 {
+       if (timeout->item.idx == UINT_MAX)
+               return;
+
        timeout_update_next(timeout, tv_now);
        if (timeout->msecs <= 1) {
                /* if we came here from io_loop_handle_timeouts(),
@@ -393,6 +420,27 @@ static void io_loop_default_time_moved(time_t old_time, time_t new_time)
        }
 }
 
+static void io_loop_timeouts_start_new(struct ioloop *ioloop)
+{
+       struct timeout *const *to_idx;
+
+       if (array_count(&ioloop->timeouts_new) == 0)
+               return;
+       
+       io_loop_time_refresh();
+
+       array_foreach(&ioloop->timeouts_new, to_idx) {
+               struct timeout *timeout= *to_idx;
+               i_assert(timeout->next_run.tv_sec == 0 &&
+                       timeout->next_run.tv_usec == 0);
+               i_assert(!timeout->one_shot);
+               i_assert(timeout->msecs > 0);
+               timeout_update_next(timeout, &ioloop_timeval);
+               priorityq_add(ioloop->timeouts, &timeout->item);
+       }
+       array_clear(&ioloop->timeouts_new);
+}
+
 static void io_loop_timeouts_update(struct ioloop *ioloop, long diff_secs)
 {
        struct priorityq_item *const *items;
@@ -545,6 +593,7 @@ static void io_loop_call_pending(struct ioloop *ioloop)
 
 void io_loop_handler_run(struct ioloop *ioloop)
 {
+       io_loop_timeouts_start_new(ioloop);
        io_loop_handler_run_internal(ioloop);
        io_loop_call_pending(ioloop);
 }
@@ -587,6 +636,7 @@ struct ioloop *io_loop_create(void)
 
         ioloop = i_new(struct ioloop, 1);
        ioloop->timeouts = priorityq_init(timeout_cmp, 32);
+       i_array_init(&ioloop->timeouts_new, 8);
 
        ioloop->time_moved_callback = current_ioloop != NULL ?
                current_ioloop->time_moved_callback :
@@ -600,6 +650,7 @@ struct ioloop *io_loop_create(void)
 void io_loop_destroy(struct ioloop **_ioloop)
 {
        struct ioloop *ioloop = *_ioloop;
+       struct timeout *const *to_idx;
        struct priorityq_item *item;
 
        *_ioloop = NULL;
@@ -622,6 +673,15 @@ void io_loop_destroy(struct ioloop **_ioloop)
        }
        i_assert(ioloop->io_pending_count == 0);
 
+       array_foreach(&ioloop->timeouts_new, to_idx) {
+               struct timeout *to = *to_idx;
+
+               i_warning("Timeout leak: %p (line %u)", (void *)to->callback,
+                         to->source_linenum);
+               timeout_free(to);
+       }
+       array_free(&ioloop->timeouts_new);
+
        while ((item = priorityq_pop(ioloop->timeouts)) != NULL) {
                struct timeout *to = (struct timeout *)item;
 

diff --git a/src/lib/test-ioloop.c b/src/lib/test-ioloop.c
index 57434981e860d727a95cbbbbd5e276c1a1c4875e..20c6207b6b2b4804d383d3d7c45d8a1c24367ecf 100644 (file)
--- a/src/lib/test-ioloop.c
+++ b/src/lib/test-ioloop.c
@@ -15,22 +15,36 @@ static void timeout_callback(struct timeval *tv)
 
 static void test_ioloop_timeout(void)
 {
-       struct ioloop *ioloop;
-       struct timeout *to;
+       struct ioloop *ioloop, *ioloop2;
+       struct timeout *to, *to2;
        struct timeval tv_start, tv_callback;
 
        test_begin("ioloop timeout");
 
        ioloop = io_loop_create();
+
+       /* add a timeout by moving it from another ioloop */
+       ioloop2 = io_loop_create();
+       to2 = timeout_add(1000, timeout_callback, &tv_callback);
+       io_loop_set_current(ioloop);
+       to2 = io_loop_move_timeout(&to2);
+       io_loop_set_current(ioloop2);
+       io_loop_destroy(&ioloop2);
+
        sleep(1);
+
+       /* add & remove immediately */
        to = timeout_add(1000, timeout_callback, &tv_callback);
        timeout_remove(&to);
+
+       /* add the timeout we're actually testing below */
        to = timeout_add(1000, timeout_callback, &tv_callback);
        if (gettimeofday(&tv_start, NULL) < 0)
                i_fatal("gettimeofday() failed: %m");
        io_loop_run(ioloop);
        test_assert(timeval_diff_msecs(&tv_callback, &tv_start) >= 500);
        timeout_remove(&to);
+       timeout_remove(&to2);
        io_loop_destroy(&ioloop);
 
        test_end();

diff --git a/src/lib/test-lib.c b/src/lib/test-lib.c
index f4e081cab10a4da6000caf60f3a1fd0038465dae..fcaebd4d7b7ec6b3cf6be82157ee1c54e70dc883 100644 (file)
--- a/src/lib/test-lib.c
+++ b/src/lib/test-lib.c
@@ -20,6 +20,7 @@ int main(void)
                test_hash_format,
                test_hash_method,
                test_hex_binary,
+               test_ioloop,
                test_iso8601_date,
                test_istream,
                test_istream_base64_decoder,

diff --git a/src/lib/test-lib.h b/src/lib/test-lib.h
index 1cd3dba0c256bc3440ea6410230f7b8f56fa66e1..23ebf9772921fd430faef440235e5157788c2a5a 100644 (file)
--- a/src/lib/test-lib.h
+++ b/src/lib/test-lib.h
@@ -21,6 +21,7 @@ void test_hash(void);
 void test_hash_format(void);
 void test_hash_method(void);
 void test_hex_binary(void);
+void test_ioloop(void);
 void test_iso8601_date(void);
 void test_istream(void);
 void test_istream_base64_decoder(void);