Re: walsender: Assert MyReplicationSlot is set before use

> On Feb 2, 2026, at 14:41, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Mon, 2 Feb 2026 at 11:55, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>>
>> Hi Hackers,
>>
>> While reviewing patch [1], I noticed that in walsender.c, the function NeedToWaitForStandbys() dereferences MyReplicationSlot->data.failover without first checking whether MyReplicationSlot is NULL.
>>
>> Looking at the call chain:
>> ```
>> logical_read_xlog_page() // XLogReaderRoutine->page_read callback
>> -> WalSndWaitForWal()
>> -> NeedToWaitForWal()
>> -> NeedToWaitForStandbys()
>> ```
>>
>> None of these callers explicitly checks whether MyReplicationSlot is NULL. Since they also don’t dereference MyReplicationSlot themselves, it wouldn’t be fair to push that responsibility up to the callers.
>>
>> Looking elsewhere in the codebase, other places that dereference MyReplicationSlot (for example CreateReplicationSlot()) either do an explicit if (MyReplicationSlot != NULL) check or assert MyReplicationSlot != NULL. So it seems reasonable to make the assumption explicit by adding an Assert(MyReplicationSlot) in NeedToWaitForStandbys().
>>
>> Another related issue is in StartLogicalReplication(): the function initially asserts MyReplicationSlot == NULL, then calls ReplicationSlotAcquire(), which is expected to set up MyReplicationSlot. Since MyReplicationSlot is dereferenced later in this function, I think it would be clearer and safer to add an Assert(MyReplicationSlot != NULL) immediately after the call to ReplicationSlotAcquire().
>>
>> The attached patch addresses both of these points.
>
> You have forgotten to attach the patch.
>
> Regards,
> Vignesh

Oops, here comes it. Thanks for the reminder.

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