From 88d9f38bab3d189fab1826b1baa9280571e39592 Mon Sep 17 00:00:00 2001 From: "Joshua D. Drake" Date: Thu, 6 Aug 2026 19:00:00 -0600 Subject: [PATCH] fix: stop the publisher cluster too, instead of only the subscriber (#470) pgc_setup installs `trap pgc_teardown EXIT`. This suite then ran trap sub_cleanup EXIT which REPLACES it rather than adding to it, so sub_cleanup stopped the subscriber and the suite's own cluster was never stopped. The suite passed and left a live postmaster behind it every run. Measured on PG17, from a box reaped to zero orphans: delta 1 before, delta 0 after, PASSED both times. Gated on PG18 and PG19, with replication.sh alongside because it is the suite that already had this right and must stay right: 20 and 41 checks, all green. Why this is not cosmetic: the port band is finite, and a suite that cannot get a port fails after 8 start attempts with "could not create any TCP/IP sockets". That is indistinguishable from a real failure, and it lands on whichever suite happened to draw the exhausted port rather than on the one that leaked. 37 orphans had accumulated on the dev box and turned two majors of a gate red for reasons that had nothing to do with the code under test. objstore_module had the identical defect and was fixed in #446. This is the last of the two the audit found. Worth noting that the audit had to be MEASURED rather than read: harness_selftest installs its own EXIT trap BEFORE pgc_setup, so by inspection pgc_setup should clobber it and leak the squatter, and it does not. Nothing detects this class of bug. A suite can leak a cluster and still report PASSED, which is why both instances survived until the band ran out. #470 records that a harness-level before/after postmaster count would catch the category. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01L2DvnWDM7g27ubDCQdXhky --- test/logical_subscriber.sh | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/logical_subscriber.sh b/test/logical_subscriber.sh index 2b6dd85..045f93c 100755 --- a/test/logical_subscriber.sh +++ b/test/logical_subscriber.sh @@ -61,7 +61,18 @@ pgc_pg "initdb -D '$SUB_DIR' -A trust" >/dev/null 2>&1 pgc_pg "printf 'port=%s\nlisten_addresses=%s\nwal_level=logical\nmax_locks_per_transaction=256\nshared_preload_libraries=%s\n' \ '$SUB_PORT' \"'127.0.0.1'\" \"'pgcolumnar'\" >> '$SUB_DIR/postgresql.conf'" pgc_pg "pg_ctl -D '$SUB_DIR' -l '$SUB_LOG' start -w" >/dev/null 2>&1 -sub_cleanup() { pgc_pg "pg_ctl -D '$SUB_DIR' -m immediate stop" >/dev/null 2>&1 || true; } +# Ends in pgc_teardown, which is not optional. pgc_setup installs +# `trap pgc_teardown EXIT`, and a second `trap ... EXIT` REPLACES it rather than +# adding to it, so this used to stop the subscriber and leave the publisher +# cluster running: measured at one orphaned postmaster per run, from a box with +# none. Orphans hold ports, the band is finite, and a suite that cannot get one +# fails after 8 start attempts with "could not create any TCP/IP sockets" -- a red +# that is indistinguishable from a real one and lands on whichever suite drew that +# port. replication.sh's sb_teardown has always chained this way (#470). +sub_cleanup() { + pgc_pg "pg_ctl -D '$SUB_DIR' -m immediate stop" >/dev/null 2>&1 || true + pgc_teardown +} trap sub_cleanup EXIT SUB() { env PATH="$PGC_BINDIR:$PATH" psql -h 127.0.0.1 -p "$SUB_PORT" -U postgres \