repack: fix uninitialized DecodingWorkerShared.initialized
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t139333psql -h localhost -U postgresBuilt from patchset v1 (message #1), July 27, 2026 at 01:44 AM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t139333_1 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t139333_1 && git checkout t139333_1Patchset v1 (message #1) is on t139333_1
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/
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).