From ea0f7e81c710328446cf8dd51de63458619697dd Mon Sep 17 00:00:00 2001 From: Joel Jacobson Date: Tue, 19 May 2026 07:36:25 -0700 Subject: [PATCH 3/3] Fix LISTEN startup race with direct advancement LISTEN creates its shared channel-map entry before commit, with listening=false until AtCommit_Notify() applies the staged action. A concurrent NOTIFY can see that commit in the window before the flag is flipped. SignalBackends() treated that backend as not interested, so direct advancement could move its queue pointer past the notification. Do not test listeners[j].listening when deciding whom to wake. A false value can mean a LISTEN that has registered its queue position, but has not yet run AtCommit_Notify(). Waking such a backend is harmless if it aborts; advancing it here could make it miss a notification after commit. The preceding missed-notification test now passes with the fix. Add a matching regression test for the fixed behavior. --- src/backend/commands/async.c | 21 ++++++---- src/test/modules/injection_points/Makefile | 1 + .../expected/async-notify-listen-race.out | 18 +++++++++ src/test/modules/injection_points/meson.build | 1 + .../specs/async-notify-listen-race.spec | 38 +++++++++++++++++++ 5 files changed, 71 insertions(+), 8 deletions(-) create mode 100644 src/test/modules/injection_points/expected/async-notify-listen-race.out create mode 100644 src/test/modules/injection_points/specs/async-notify-listen-race.spec diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c index bd7eff7f305..f1bd278bd19 100644 --- a/src/backend/commands/async.c +++ b/src/backend/commands/async.c @@ -114,11 +114,11 @@ * If the current transaction has executed any LISTEN/UNLISTEN actions, * PreCommit_Notify() prepares to commit those. For LISTEN, it * pre-allocates entries in both the per-backend localChannelTable and the - * shared globalChannelTable (with listening=false so that these entries - * are no-ops for the moment). It also records the final per-channel - * intent in pendingListenActions, so post-commit/abort processing can - * apply that in a single step. Since all these allocations happen before - * committing to clog, we can safely abort the transaction on failure. + * shared globalChannelTable (with listening=false to mark these entries + * as staged). It also records the final per-channel intent in + * pendingListenActions, so post-commit/abort processing can apply that in + * a single step. Since all these allocations happen before committing to + * clog, we can safely abort the transaction on failure. * * After commit, AtCommit_Notify() runs through pendingListenActions and * updates the backend's per-channel listening flags to activate or @@ -2311,9 +2311,6 @@ SignalBackends(void) int32 pid; QueuePosition pos; - if (!listeners[j].listening) - continue; /* ignore not-yet-committed listeners */ - i = listeners[j].procNo; if (QUEUE_BACKEND_WAKEUP_PENDING(i)) @@ -2327,6 +2324,14 @@ SignalBackends(void) Assert(pid != InvalidPid); + /* + * Do not test listeners[j].listening here. A false value can + * mean a LISTEN that has registered its queue position, but has + * not yet run AtCommit_Notify(). Waking such a backend is + * harmless if it aborts; advancing it here could make it miss a + * notification after commit. + */ + QUEUE_BACKEND_WAKEUP_PENDING(i) = true; signalPids[count] = pid; signalProcnos[count] = i; diff --git a/src/test/modules/injection_points/Makefile b/src/test/modules/injection_points/Makefile index dcf442df9ba..665e7852d52 100644 --- a/src/test/modules/injection_points/Makefile +++ b/src/test/modules/injection_points/Makefile @@ -15,6 +15,7 @@ REGRESS_OPTS = --dlpath=$(top_builddir)/src/test/regress ISOLATION = basic \ async-notify-listen-false-positive \ async-notify-listen-startup \ + async-notify-listen-race \ inplace \ repack \ repack_temporal \ diff --git a/src/test/modules/injection_points/expected/async-notify-listen-race.out b/src/test/modules/injection_points/expected/async-notify-listen-race.out new file mode 100644 index 00000000000..d65e5015cff --- /dev/null +++ b/src/test/modules/injection_points/expected/async-notify-listen-race.out @@ -0,0 +1,18 @@ +Parsed test spec with 3 sessions + +starting permutation: listen notify wake check +step listen: LISTEN race; +step notify: NOTIFY race, 'payload'; +step wake: + SELECT FROM injection_points_detach('async-notify-before-listen-commit'); + SELECT FROM injection_points_wakeup('async-notify-before-listen-commit'); + +step listen: <... completed> +listener: NOTIFY "race" with payload "payload" from notifier +step check: SELECT 1 AS x; +x +- +1 +(1 row) + +step wake: <... completed> diff --git a/src/test/modules/injection_points/meson.build b/src/test/modules/injection_points/meson.build index 8a91b0a41aa..7fb8de3d0f8 100644 --- a/src/test/modules/injection_points/meson.build +++ b/src/test/modules/injection_points/meson.build @@ -46,6 +46,7 @@ tests += { 'basic', 'async-notify-listen-false-positive', 'async-notify-listen-startup', + 'async-notify-listen-race', 'inplace', 'repack', 'repack_temporal', diff --git a/src/test/modules/injection_points/specs/async-notify-listen-race.spec b/src/test/modules/injection_points/specs/async-notify-listen-race.spec new file mode 100644 index 00000000000..3dd6bcfa8b0 --- /dev/null +++ b/src/test/modules/injection_points/specs/async-notify-listen-race.spec @@ -0,0 +1,38 @@ +# Test LISTEN/NOTIFY race around the LISTEN transaction's commit. +# +# A LISTEN backend registers its queue position before commit, but only applies +# local listen state in AtCommit_Notify(). A concurrent NOTIFY that commits +# while the LISTEN backend is between those two points must not advance the +# listener past the notification. + +setup +{ + CREATE EXTENSION injection_points; +} + +teardown +{ + DROP EXTENSION injection_points; +} + +session listener +setup +{ + SELECT FROM injection_points_set_local(); + SELECT FROM injection_points_attach('async-notify-before-listen-commit', 'wait'); +} +step listen { LISTEN race; } +step check { SELECT 1 AS x; } +teardown { UNLISTEN *; } + +session notifier +step notify { NOTIFY race, 'payload'; } + +session controller +step wake +{ + SELECT FROM injection_points_detach('async-notify-before-listen-commit'); + SELECT FROM injection_points_wakeup('async-notify-before-listen-commit'); +} + +permutation listen notify wake(listen) check -- 2.52.0