isc_async_run() runs events in reverse order

when more than one event was scheduled in the isc_aysnc queue,
they were executed in reverse order. we need to pull events
off the back of queue instead the front, so that uv_loop will
run them in the right order.

note that isc_job_run() has the same behavior, because it calls
uv_idle_start() directly. in that case we just document it so
it'll be less surprising in the future.

diff --git a/lib/isc/async.c b/lib/isc/async.c
index 03e2cbd799f3ee8c8ceb13078cddb2c79bd72bd9..a4d08eaadf2b626526b0ab1484a126b18677b8cd 100644 (file)
--- a/lib/isc/async.c
+++ b/lib/isc/async.c
@@ -47,7 +47,9 @@ isc_async_run(isc_loop_t *loop, isc_job_cb cb, void *cbarg) {
 
        job = isc__job_new(loop, cb, cbarg);
 
-       /* Now send the half initialized job to loop queue */
+       /*
+        * Now send the half-initialized job to the loop queue.
+        */
        LOCK(&loop->queue_lock);
        ISC_LIST_APPEND(loop->queue_jobs, job, link);
        UNLOCK(&loop->queue_lock);

diff --git a/lib/isc/include/isc/job.h b/lib/isc/include/isc/job.h
index 2c75e3042cb74fbf61c64aea4965d5827205768b..19ab75d724d3ffeda26650cd26354eb9c5c50133 100644 (file)
--- a/lib/isc/include/isc/job.h
+++ b/lib/isc/include/isc/job.h
@@ -39,6 +39,11 @@ isc_job_run(isc_loopmgr_t *loopmgr, isc_job_cb cb, void *cbarg);
  * Schedule the job callback 'cb' to be run on the currently
  * running event loop.
  *
+ * Note: Because of the design of uv_idle_start(), if more than one
+ * job is posted at once, the jobs will be pushed onto a stack and
+ * executed in last-in-first-out order. To post events that are
+ * executed in order posted, use isc_async_run() instead.
+ *
  * Requires:
  *
  *\li  'loopmgr' is the active loop manager.

diff --git a/lib/isc/loop.c b/lib/isc/loop.c
index 52817be01c351765da2e4bd6d3e1cbb2ae629b79..b77ba534f8b8b3d756357d07920de5048560f050 100644 (file)
--- a/lib/isc/loop.c
+++ b/lib/isc/loop.c
@@ -191,9 +191,14 @@ queue_cb(uv_async_t *handle) {
        ISC_LIST_MOVE(list, loop->queue_jobs);
        UNLOCK(&loop->queue_lock);
 
-       job = ISC_LIST_HEAD(list);
+       /*
+        * The ISC_LIST_TAIL is counterintuitive here, but uv_idle
+        * drains its queue backwards, so if there's more than one event to
+        * be processed then they need to be in reverse order.
+        */
+       job = ISC_LIST_TAIL(list);
        while (job != NULL) {
-               isc_job_t *next = ISC_LIST_NEXT(job, link);
+               isc_job_t *next = ISC_LIST_PREV(job, link);
                ISC_LIST_UNLINK(list, job, link);
 
                isc__job_init(loop, job);

diff --git a/tests/isc/async_test.c b/tests/isc/async_test.c
index 138147b3e074401821c07b3996d1f733e0c07cb3..41f0ee18f50c5ddf3b41a01eaf4654cb559ce30e 100644 (file)
--- a/tests/isc/async_test.c
+++ b/tests/isc/async_test.c
@@ -39,9 +39,10 @@ static atomic_uint scheduled = 0;
 
 static void
 async_cb(void *arg) {
-       UNUSED(arg);
        uint32_t tid = isc_tid();
 
+       UNUSED(arg);
+
        atomic_fetch_add(&scheduled, 1);
 
        if (tid > 0) {
@@ -54,11 +55,11 @@ async_cb(void *arg) {
 
 static void
 async_setup_cb(void *arg) {
-       UNUSED(arg);
        uint32_t tid = isc_loopmgr_nloops(loopmgr) - 1;
-
        isc_loop_t *loop = isc_loop_get(loopmgr, tid);
 
+       UNUSED(arg);
+
        isc_async_run(loop, async_cb, loopmgr);
 }
 
@@ -68,8 +69,40 @@ ISC_RUN_TEST_IMPL(isc_async_run) {
        assert_int_equal(atomic_load(&scheduled), loopmgr->nloops);
 }
 
+static char string[32] = "";
+int n1 = 1, n2 = 2, n3 = 3, n4 = 4, n5 = 5;
+
+static void
+append(void *arg) {
+       char value[32];
+       sprintf(value, "%d", *(int *)arg);
+       strlcat(string, value, 10);
+}
+
+static void
+async_multiple(void *arg) {
+       isc_loop_t *loop = isc_loop_current(loopmgr);
+
+       UNUSED(arg);
+
+       isc_async_run(loop, append, &n1);
+       isc_async_run(loop, append, &n2);
+       isc_async_run(loop, append, &n3);
+       isc_async_run(loop, append, &n4);
+       isc_async_run(loop, append, &n5);
+       isc_loopmgr_shutdown(loopmgr);
+}
+
+ISC_RUN_TEST_IMPL(isc_async_multiple) {
+       string[0] = '\0';
+       isc_loop_setup(isc_loop_main(loopmgr), async_multiple, loopmgr);
+       isc_loopmgr_run(loopmgr);
+       assert_string_equal(string, "12345");
+}
+
 ISC_TEST_LIST_START
 ISC_TEST_ENTRY_CUSTOM(isc_async_run, setup_loopmgr, teardown_loopmgr)
+ISC_TEST_ENTRY_CUSTOM(isc_async_multiple, setup_loopmgr, teardown_loopmgr)
 ISC_TEST_LIST_END
 
 ISC_TEST_MAIN

diff --git a/tests/isc/job_test.c b/tests/isc/job_test.c
index 4ac4c0a9035f23440dca2420d2dbfd65f4bd33a7..2a8c049bed795ecf7fbc3311de374f96ff2701f1 100644 (file)
--- a/tests/isc/job_test.c
+++ b/tests/isc/job_test.c
@@ -76,8 +76,39 @@ ISC_RUN_TEST_IMPL(isc_job_run) {
        assert_int_equal(atomic_load(&scheduled), atomic_load(&executed));
 }
 
+static char string[32] = "";
+int n1 = 1, n2 = 2, n3 = 3, n4 = 4, n5 = 5;
+
+static void
+append(void *arg) {
+       char value[32];
+       sprintf(value, "%d", *(int *)arg);
+       strlcat(string, value, 10);
+}
+
+static void
+job_multiple(void *arg) {
+       UNUSED(arg);
+
+       /* These will be processed in reverse order */
+       isc_job_run(loopmgr, append, &n1);
+       isc_job_run(loopmgr, append, &n2);
+       isc_job_run(loopmgr, append, &n3);
+       isc_job_run(loopmgr, append, &n4);
+       isc_job_run(loopmgr, append, &n5);
+       isc_loopmgr_shutdown(loopmgr);
+}
+
+ISC_RUN_TEST_IMPL(isc_job_multiple) {
+       string[0] = '\0';
+       isc_loop_setup(isc_loop_main(loopmgr), job_multiple, loopmgr);
+       isc_loopmgr_run(loopmgr);
+       assert_string_equal(string, "54321");
+}
+
 ISC_TEST_LIST_START
 ISC_TEST_ENTRY_CUSTOM(isc_job_run, setup_loopmgr, teardown_loopmgr)
+ISC_TEST_ENTRY_CUSTOM(isc_job_multiple, setup_loopmgr, teardown_loopmgr)
 ISC_TEST_LIST_END
 
 ISC_TEST_MAIN