public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: Postgres hackers <[email protected]>
Subject: repack: fix uninitialized DecodingWorkerShared.initialized
Date: Wed, 15 Apr 2026 18:22:44 +0800
Message-ID: <[email protected]> (raw)

Hi,

I have not reviewed the repack-related patches before. Recently, I started trying to understand how repack works and trace through the code.

While tracing start_repack_decoding_worker(), I noticed something suspicious:

```
	seg = dsm_create(size, 0);
	shared = (DecodingWorkerShared *) dsm_segment_address(seg);
	shared->lsn_upto = InvalidXLogRecPtr;
	shared->done = false;
	SharedFileSetInit(&shared->sfs, seg);
	shared->last_exported = -1;
	SpinLockInit(&shared->mutex);
	shared->dbid = MyDatabaseId;
```

Here, the code creates a shared-memory segment and lets “shared" point to that memory. It then initializes some fields of “shared". However, later code reads shared->initialized, but this field was not initialized:
```
	for (;;)
	{
		bool		initialized;

		SpinLockAcquire(&shared->mutex);
		initialized = shared->initialized;
		SpinLockRelease(&shared->mutex);

		if (initialized)
			break;

		ConditionVariableSleep(&shared->cv, WAIT_EVENT_REPACK_WORKER_EXPORT);
	}
```

I checked the code of dsm_create(), and I did not see anything showing that the created shared-memory segment is zeroed.

This is actually just an eyeball finding. From my tracing on my MacBook, after shared = (DecodingWorkerShared *) dsm_segment_address(seg);, the memory always seemed to be zeroed, so I may have missed something. But given that the code explicitly initializes several other fields, it seems better not to rely on that implicitly.

For the fix, since start_repack_decoding_worker() is not on a hot path, I think it is fine to zero the whole shared struct explicitly, and then initialize the non-zero fields afterwards.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Attachments:

  [application/octet-stream] v1-0001-repack-zero-initialize-DecodingWorkerShared.patch (1010B, 2-v1-0001-repack-zero-initialize-DecodingWorkerShared.patch)
  download | inline diff:
From b9c22b95492bff66a511159ceffd37d6394cc5b1 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Wed, 15 Apr 2026 16:46:20 +0800
Subject: [PATCH v1] repack: zero-initialize DecodingWorkerShared

Author: Chao Li <[email protected]>
Reviewed-by:
Discussion: https://postgr.es/m/
---
 src/backend/commands/repack.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index 58e3867246f..058b39a67ad 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -3311,8 +3311,7 @@ start_repack_decoding_worker(Oid relid)
 		BUFFERALIGN(REPACK_ERROR_QUEUE_SIZE);
 	seg = dsm_create(size, 0);
 	shared = (DecodingWorkerShared *) dsm_segment_address(seg);
-	shared->lsn_upto = InvalidXLogRecPtr;
-	shared->done = false;
+	memset(shared, 0, sizeof(DecodingWorkerShared));
 	SharedFileSetInit(&shared->sfs, seg);
 	shared->last_exported = -1;
 	SpinLockInit(&shared->mutex);
-- 
2.50.1 (Apple Git-155)

view thread (4+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: repack: fix uninitialized DecodingWorkerShared.initialized
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox