Re: repack: fix uninitialized DecodingWorkerShared.initialized

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] https://www.postgresql.org/message-id/182883.1776073323%40localhost

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com