From 3b033fa79e3963cbdeb2300508164d00af61f124 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 5 Aug 2025 14:35:09 +1200 Subject: [PATCH] Fix bug in read_stream.c's split IO handling. If a circular queue wraparound, a multi-block IO split and a transition to the fast path happened in a certain sequence, a buffer forwarded from one StartReadBuffers() call to next would not be cleared out. cleared out. That could confuse a later queue-wrapping StartReadBuffers() call by passing it a random unpinned buffer. Fix, and add some tighter and earlier assertions about the layout and identity of buffers forwarded across IO splits, and the following entries that should have been cleared before reuse. Defect in commit ed0b87ca. Bug: 19006 Backpatch-through: 18 Reported-by: Alexander Lakhin Discussion: https://postgr.es/m/19006-80fcaaf69000377e%40postgresql.org --- src/backend/storage/aio/read_stream.c | 32 +++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/backend/storage/aio/read_stream.c b/src/backend/storage/aio/read_stream.c index 0e7f5557f5c..a9cfe3347c8 100644 --- a/src/backend/storage/aio/read_stream.c +++ b/src/backend/storage/aio/read_stream.c @@ -253,6 +253,25 @@ read_stream_start_pending_read(ReadStream *stream) else Assert(stream->next_buffer_index == stream->oldest_buffer_index); + /* + * Pinned buffers forwarded by a preceding StartReadBuffers() call that + * had to split the operation should match the leading blocks of this + * following StartReadBuffers() call. + */ + Assert(stream->forwarded_buffers <= stream->pending_read_nblocks); + for (int i = 0; i < stream->forwarded_buffers; ++i) + Assert(BufferGetBlockNumber(stream->buffers[stream->next_buffer_index + i]) == + stream->pending_read_blocknum + i); + + /* + * Check that we've cleared the queue/overflow entries corresponding to + * the rest of the blocks covered by this read, unless it's the first go + * around and we haven't even initialized them yet. + */ + for (int i = stream->forwarded_buffers; i < stream->pending_read_nblocks; ++i) + Assert(stream->next_buffer_index + i >= stream->initialized_buffers || + stream->buffers[stream->next_buffer_index + i] == InvalidBuffer); + /* Do we need to issue read-ahead advice? */ flags = stream->read_buffers_flags; if (stream->advice_enabled) @@ -979,6 +998,19 @@ read_stream_next_buffer(ReadStream *stream, void **per_buffer_data) stream->pending_read_nblocks == 0 && stream->per_buffer_data_size == 0) { + /* + * The fast path spins on one buffer entry repeatedly instead of + * rotating through the whole queue and clearing the entries behind + * it. If the buffer it starts with happened to be forwarded between + * StartReadBuffers() calls and also wrapped around the circular queue + * partway through, then a copy also exists in the overflow zone, and + * it won't clear it out as the regular path would. Do that now, so + * it doesn't need code for that. + */ + if (stream->oldest_buffer_index < stream->io_combine_limit - 1) + stream->buffers[stream->queue_size + stream->oldest_buffer_index] = + InvalidBuffer; + stream->fast_path = true; } #endif -- 2.47.2