From: Timo Sirainen <tss@iki.fi>
Date: Thu, 27 Aug 2015 15:34:52 +0000 (+0200)
Subject: lib: Try fixing ec6e672a6e32 (ioloop timeout fixing)
X-Git-Tag: 2.2.19.rc1~169
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=9b3565b09683b48f66de51aebb52786934d1c324;p=thirdparty%2Fdovecot%2Fcore.git

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 901a56f626..c13b4ce7a3 100644
--- 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 b149f0c470..9fbb18c252 100644
--- 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 70d064f827..87e452491a 100644
--- 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 57434981e8..20c6207b6b 100644
--- 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 f4e081cab1..fcaebd4d7b 100644
--- 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 1cd3dba0c2..23ebf97729 100644
--- 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);