BUG/MINOR: ring/cli: fix a race condition between the writer and the reader

The ring's CLI reader unlocks the read side of a ring and relocks it for
writing only if it needs to re-subscribe. But during this time, the writer
might have pushed data, see nobody subscribed hence woken nobody, while
the reader would have left marking that the applet had no more data. This
results in a dump that will not make any forward progress: the ring is
clogged by this reader which believes there's no data and the writer
will never wake it up.

The right approach consists in verifying after re-attaching if the writer
had made any progress in between, and to report that another call is
needed. Note that a jump back to the beginning would also work but here
we provide better fairness between readers this way.

This needs to be backported to 2.2. The applet API needed to signal the
availability of new data changed a few times since then.

diff --git a/src/ring.c b/src/ring.c
index 7c36f7bef49c68bf0bbc0539fa548aa650cf7c5e..c45de29eb1a1dc689e993ce0128ccaa46c8ca7ee 100644 (file)
--- a/src/ring.c
+++ b/src/ring.c
@@ -294,6 +294,7 @@ int cli_io_handler_show_ring(struct appctx *appctx)
        struct ring *ring = ctx->ring;
        struct buffer *buf = &ring->buf;
        size_t ofs = ctx->ofs;
+       size_t last_ofs;
        uint64_t msg_len;
        size_t len, cnt;
        int ret;
@@ -366,6 +367,7 @@ int cli_io_handler_show_ring(struct appctx *appctx)
 
        HA_ATOMIC_INC(b_peek(buf, ofs));
        ofs += ring->ofs;
+       last_ofs = ring->ofs;
        ctx->ofs = ofs;
        HA_RWLOCK_RDUNLOCK(LOGSRV_LOCK, &ring->lock);
 
@@ -377,8 +379,16 @@ int cli_io_handler_show_ring(struct appctx *appctx)
                        /* let's be woken up once new data arrive */
                        HA_RWLOCK_WRLOCK(LOGSRV_LOCK, &ring->lock);
                        LIST_APPEND(&ring->waiters, &appctx->wait_entry);
+                       ofs = ring->ofs;
                        HA_RWLOCK_WRUNLOCK(LOGSRV_LOCK, &ring->lock);
-                       applet_have_no_more_data(appctx);
+                       if (ofs != last_ofs) {
+                               /* more data was added into the ring between the
+                                * unlock and the lock, and the writer might not
+                                * have seen us. We need to reschedule a read.
+                                */
+                               applet_have_more_data(appctx);
+                       } else
+                               applet_have_no_more_data(appctx);
                        ret = 0;
                }
                /* always drain all the request */