repack: fix uninitialized DecodingWorkerShared.initialized
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:
v1-0001-repack-zero-initialize-DecodingWorkerShared.patchapplication/octet-stream; name=v1-0001-repack-zero-initialize-DecodingWorkerShared.patch; x-unix-mode=0644Download+1-3
Chao Li <li.evan.chao@gmail.com> wrote:
I have not reviewed the repack-related patches before. Recently, I started trying to understand how repack works and trace through the code.
Thanks, I appreciate that!
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:
The problem was noticed earlier this week and I already posted a fix [1]/messages/by-id/182883.1776073323@localhost.
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.
Although not strongly, I prefer setting individual fields explicitly.
[1]: /messages/by-id/182883.1776073323@localhost
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
On Apr 15, 2026, at 23:07, Antonin Houska <ah@cybertec.at> wrote:
Chao Li <li.evan.chao@gmail.com> wrote:
I have not reviewed the repack-related patches before. Recently, I started trying to understand how repack works and trace through the code.
Thanks, I appreciate that!
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:
The problem was noticed earlier this week and I already posted a fix [1].
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.
Although not strongly, I prefer setting individual fields explicitly.
[1] /messages/by-id/182883.1776073323@localhost
--
Antonin Houska
Web: https://www.cybertec-postgresql.com
I saw 05c401d5786a05ea630e884ffa492aa01683d15b has fixed this issue, so this patch is no longer needed.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Thu, Apr 16, 2026 at 2:17 AM Chao Li <li.evan.chao@gmail.com> wrote:
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 record, that depends on whether you get a newly allocated
shared memory segment from the operating system, or recycled shared
memory reserved at startup with the min_dynamic_shared_memory setting
(as shown in Alexander Lakhin's reproducer).