This is an automated email from the ASF dual-hosted git repository. reshke pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit f83630396e011c210125266d0647811311e4dd44 Author: Heikki Linnakangas <[email protected]> AuthorDate: Mon Jul 25 08:48:38 2022 +0300 Fix ReadRecentBuffer for local buffers. It incorrectly used GetBufferDescriptor instead of GetLocalBufferDescriptor, causing it to not find the correct buffer in most cases, and performing an out-of-bounds memory read in the corner case that temp_buffers > shared_buffers. It also bumped the usage-count on the buffer, even if it was previously pinned. That won't lead to crashes or incorrect results, but it's different from what the shared-buffer case does, and different from the usual code in LocalBufferAlloc. Fix that too, and make the code ordering match LocalBufferAlloc() more closely, so that it's easier to verify that it's doing the same thing. Currently, ReadRecentBuffer() is only used with non-temp relations, in WAL redo, so the broken code is currently dead code. However, it could be used by extensions. Backpatch-through: 14 Discussion: https://www.postgresql.org/message-id/2d74b46f-27c9-fb31-7f99-327a87184cc0%40iki.fi Reviewed-by: Thomas Munro, Zhang Mingli, Richard Guo --- src/backend/storage/buffer/bufmgr.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 6d0afd34356..dd16c3df60a 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -718,18 +718,28 @@ ReadRecentBuffer(RelFileNode rnode, ForkNumber forkNum, BlockNumber blockNum, if (BufferIsLocal(recent_buffer)) { - bufHdr = GetBufferDescriptor(-recent_buffer - 1); + int b = -recent_buffer - 1; + + bufHdr = GetLocalBufferDescriptor(b); buf_state = pg_atomic_read_u32(&bufHdr->state); /* Is it still valid and holding the right tag? */ if ((buf_state & BM_VALID) && BUFFERTAGS_EQUAL(tag, bufHdr->tag)) { - /* Bump local buffer's ref and usage counts. */ + /* + * Bump buffer's ref and usage counts. This is equivalent of + * PinBuffer for a shared buffer. + */ + if (LocalRefCount[b] == 0) + { + if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT) + { + buf_state += BUF_USAGECOUNT_ONE; + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + } + } + LocalRefCount[b]++; ResourceOwnerRememberBuffer(CurrentResourceOwner, recent_buffer); - LocalRefCount[-recent_buffer - 1]++; - if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT) - pg_atomic_write_u32(&bufHdr->state, - buf_state + BUF_USAGECOUNT_ONE); return true; } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
