| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | walsender: Assert MyReplicationSlot is set before use |
| Date: | 2026-02-02 06:25:01 |
| Message-ID: | 6E7BD4F7-C22A-4B6C-A9BD-62877390DF86@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
[1] https://postgr.es/m/CAOzEurSRii0tEYhu5cePmRcvS=ZrxTLEvxm3Kj0d7_uKGdM23g@mail.gmail.com
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2026-02-02 06:29:20 | Re: Wake up backends immediately when sync standbys decrease |
| Previous Message | shveta malik | 2026-02-02 06:12:21 | Re: Skipping schema changes in publication |