io: fix nasty io_wake corner case.

If we're duplex and one io_always callback makes the other io_always,
we screwed up and hit an assertion later when the conn was in the
always list but didn't actually want to be.

io_wake() uses io_always(), so this is how it happened.  Writing a
test case for this was a bit fun, too.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/ccan/io/io.c b/ccan/io/io.c
index f298af70781d3fe90e606934614f5d06874e770f..12b2489023d94077d5562db876ab9004caae3dd4 100644 (file)
--- a/ccan/io/io.c
+++ b/ccan/io/io.c
@@ -392,12 +392,25 @@ void io_ready(struct io_conn *conn, int pollflags)
 
 void io_do_always(struct io_conn *conn)
 {
+       /* There's a corner case where the in next_plan wakes up the
+        * out, placing it in IO_ALWAYS and we end up processing it immediately,
+        * only to leave it in the always list.
+        *
+        * Yet we can't just process one, in case they are both supposed
+        * to be done, so grab state beforehand.
+        */
+       bool always_out = (conn->plan[IO_OUT].status == IO_ALWAYS);
+
        if (conn->plan[IO_IN].status == IO_ALWAYS)
                if (!next_plan(conn, &conn->plan[IO_IN]))
                        return;
 
-       if (conn->plan[IO_OUT].status == IO_ALWAYS)
+       if (always_out) {
+               /* You can't *unalways* a conn (except by freeing, in which
+                * case next_plan() returned false */
+               assert(conn->plan[IO_OUT].status == IO_ALWAYS);
                next_plan(conn, &conn->plan[IO_OUT]);
+       }
 }
 
 void io_do_wakeup(struct io_conn *conn, enum io_direction dir)

diff --git a/ccan/io/test/run-40-wakeup-mutual.c b/ccan/io/test/run-40-wakeup-mutual.c
new file mode 100644 (file)
index 0000000..04ec944
--- /dev/null
+++ b/ccan/io/test/run-40-wakeup-mutual.c
@@ -0,0 +1,54 @@
+/* Test previous issue: in duplex case, we wake reader reader wakes writer. */
+#include <ccan/io/io.h>
+/* Include the C files directly. */
+#include <ccan/io/poll.c>
+#include <ccan/io/io.c>
+#include <ccan/tap/tap.h>
+#include <sys/wait.h>
+#include <stdio.h>
+
+static struct io_plan *block_reading(struct io_conn *conn, void *unused)
+{
+       static char buf[1];
+       return io_read(conn, buf, sizeof(buf), io_never, NULL);
+}
+
+static struct io_plan *writer_woken(struct io_conn *conn, void *unused)
+{
+       pass("Writer woken up");
+       return io_write(conn, "1", 1, io_close_cb, NULL);
+}
+
+static struct io_plan *reader_woken(struct io_conn *conn, void *unused)
+{
+       pass("Reader woken up");
+       /* Wake writer */
+       io_wake(conn);
+       return block_reading(conn, unused);
+}
+
+static struct io_plan *setup_conn(struct io_conn *conn, void *trigger)
+{
+       return io_duplex(conn,
+                        io_wait(conn, trigger, reader_woken, NULL),
+                        io_out_wait(conn, conn, writer_woken, NULL));
+}
+
+int main(void)
+{
+       int fds[2];
+       
+       plan_tests(3);
+       ok1(socketpair(AF_LOCAL, SOCK_STREAM, 0, fds) == 0);
+
+       /* We use 'fds' as pointer to wake writer. */
+       io_new_conn(NULL, fds[0], setup_conn, fds);
+
+       io_wake(fds);
+       io_loop(NULL, NULL);
+
+       close(fds[1]);
+
+       /* This exits depending on whether all tests passed */
+       return exit_status();
+}