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.
This commit is contained in:
Willy Tarreau 2022-08-04 17:00:21 +02:00
parent 48bb875908
commit b8e0fb97f3
1 changed files with 11 additions and 1 deletions

View File

@ -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 */