Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 17 Jun 2026 13:15:13 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Cc: zlh21343@163.com, pgsql-bugs@lists.postgresql.org,
	Andres Freund <andres@anarazel.de>,
	Sami Imseih <samimseih@gmail.com>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Subject: Re: BUG #19520: PANIC when concurrently manipulating stored
 procedures with pg_stat_statements and track_functions =
Message-ID: <ajIfUavZvZqep3EQ@paquier.xyz>
References: <19520-73873648d44793cf@postgresql.org>
 <CAJTYsWVqtWF+KJUdoKrzUM9QPQA5qD225AoeBeN7cwT4V1qd6A@mail.gmail.com>
 <ajCJw956W3xsl8XP@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="j9H3Lna40EX9MUWT"
Content-Disposition: inline
In-Reply-To: <ajCJw956W3xsl8XP@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/ajIfUavZvZqep3EQ%40paquier.xyz>
Precedence: bulk


--j9H3Lna40EX9MUWT
Content-Type: multipart/mixed; boundary="wVmyIxSo9dssAgWx"
Content-Disposition: inline


--wVmyIxSo9dssAgWx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Jun 16, 2026 at 08:24:51AM +0900, Michael Paquier wrote:
> On Mon, Jun 15, 2026 at 02:44:06PM +0530, Ayush Tiwari wrote:
>> I've added Andres and Michael on the thread, since they have worked on
>> this in the past, for their input.
>=20
> Thanks for the poke.  I have marked this thread as something to look
> at, but was not able to get back to it.  Will investigate..

As far as I can see, pgss is not really a requirement.  Your case is
taking advantage of the module introducing more slowness to enlarge
the reproduction window.  Now saying that pgss being slow is a good
thing, it's bad, but it helps here.  I've tried to reproduce in three
environments, only my mac is able to get something, because it's
slower I guess..

Attached is a script able to reproduce the issue in bash, courtesy of
Claude because java and I sum up to a value very close to 0, see
test_bug19520.txt.  The trick of the script is the same as your
scenario, with two concurrent workloads:
- One with DROP PROC/CREATE PROC/CALL.
- One with CALL

I had much more success after adding two sleeps to enlarge the
conflict window, see also the sleep.patch attached, for reference.

Finally attached is a patch, where I'd like to propose the
introduction of a path in pgstat_drop_entry() to make the routine able
to accept double drops.

The big comment within pgstat_init_function_usage() documents why it
does its stuff for track_functions, so I was wondering if we should
enforce the same double-drop-acceptance rule for all the callers
everybody, but I also see a point in the correctness, by allowing the
caller to complain if we try to do double drops but error on them,
pointing to a programming error.  Note that
pgstat_drop_entry_internal() is not touched on purpose, to keep the
database-level scans as they are, with double-drops forbidden.

This patch is very close to what Sami has posted on his PGSS thread,
v3-0002, using a missing_ok instead of a skip_dropped:
https://www.postgresql.org/message-id/CAA5RZ0uoxiQ2_=3DxHGRnyc4WdM9aR0fzdMh=
Bubnw97po=3D=3D--yGQ@mail.gmail.com
I didn't suspect that we would need something like that for a
backpatch, but well.

I'm adding Sami in CC in case he wishes to comment on this patch, and
Horiguchi-san as this area of the code concerns him.

Thoughts or comments welcome.
--
Michael

--wVmyIxSo9dssAgWx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=sleep.patch

diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c
index d47d05e3d922..eef97a29de7c 100644
--- a/src/backend/utils/activity/pgstat_function.c
+++ b/src/backend/utils/activity/pgstat_function.c
@@ -112,6 +112,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
 		AcceptInvalidationMessages();
 		if (!SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(fcinfo->flinfo->fn_oid)))
 		{
+			pg_usleep(10000);	/* 10ms */
 			pgstat_drop_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId,
 							  fcinfo->flinfo->fn_oid);
 			ereport(ERROR, errcode(ERRCODE_UNDEFINED_FUNCTION),
diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/activity/pgstat_xact.c
index 5e2d69e62979..70a1e302514f 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -85,6 +85,8 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_state, bool isCommit)
 			 * Transaction that dropped an object committed. Drop the stats
 			 * too.
 			 */
+			if (it->kind == PGSTAT_KIND_FUNCTION)
+				pg_usleep(500000);	/* 500ms */
 			if (!pgstat_drop_entry(it->kind, it->dboid, objid))
 				not_freed_count++;
 		}

--wVmyIxSo9dssAgWx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=test_bug19520.txt

#!/bin/bash
#
# Reproducer for BUG #19520: PANIC when concurrently manipulating
# stored procedures with track_functions = all (or pl).
#
# On an unfixed build this PANICs. On a fixed build the server survives.

set -e

PGPORT="${1:-5432}"
PGDATABASE="postgres"
PGUSER="${USER:-postgres}"
N_DDL=10          # number of DROP/CREATE/CALL workers
N_CALL=20         # number of CALL-only workers
ITERATIONS=3000   # iterations per worker

export PGPORT PGDATABASE PGUSER

echo "=== BUG #19520 reproducer ==="
echo "Port: $PGPORT | DDL workers: $N_DDL | CALL workers: $N_CALL"
echo "Iterations per worker: $ITERATIONS"
echo ""

# Verify the server is reachable
if ! psql -Xc "SELECT 1" > /dev/null 2>&1; then
    echo "ERROR: Cannot connect to PostgreSQL on port $PGPORT."
    echo "Make sure the server is running with:"
    echo "  track_functions = 'all'  (or 'pl')"
    exit 1
fi

# Check track_functions setting
TF=$(psql -XtAc "SHOW track_functions")
if [ "$TF" = "none" ]; then
    echo "ERROR: track_functions = 'none'. Must be 'pl' or 'all'."
    exit 1
fi
echo "track_functions = '$TF'"

# Create the initial procedure
psql -Xc "CREATE OR REPLACE PROCEDURE proc_race_test()
           LANGUAGE plpgsql AS \$\$ BEGIN END; \$\$;" 2>/dev/null || true

PIDS=()

# Generate a SQL script for DDL workers (persistent connection, tight loop)
DDL_SQL=$(mktemp)
for j in $(seq 1 $ITERATIONS); do
    cat >> "$DDL_SQL" <<'EOF'
DROP PROCEDURE IF EXISTS proc_race_test;
CREATE OR REPLACE PROCEDURE proc_race_test() LANGUAGE plpgsql AS $$ BEGIN END; $$;
CALL proc_race_test();
EOF
done

# Generate a SQL script for CALL workers (persistent connection, tight loop)
CALL_SQL=$(mktemp)
for j in $(seq 1 $ITERATIONS); do
    echo "CALL proc_race_test();" >> "$CALL_SQL"
done

# Launch DDL workers using persistent psql sessions
for i in $(seq 1 $N_DDL); do
    psql -X -f "$DDL_SQL" > /dev/null 2>&1 &
    PIDS+=($!)
done

# Launch CALL-only workers using persistent psql sessions
for i in $(seq 1 $N_CALL); do
    psql -X -f "$CALL_SQL" > /dev/null 2>&1 &
    PIDS+=($!)
done

echo "Launched ${#PIDS[@]} background workers (persistent connections). Waiting..."
echo "(On an unfixed build the server will PANIC; on a fixed build it survives.)"
echo ""

# Wait for all workers
for pid in "${PIDS[@]}"; do
    wait "$pid" 2>/dev/null || true
done

# Cleanup temp files
rm -f "$DDL_SQL" "$CALL_SQL"

echo ""
echo "All workers finished."

# Check if the server is still alive
if psql -Xc "SELECT 1" > /dev/null 2>&1; then
    echo "PASS: Server is still running - no PANIC occurred."
    exit 0
else
    echo "FAIL: Server is down - likely hit the PANIC."
    echo "Check the server log for:"
    echo '  PANIC: cannot abort transaction xxx, it was already committed'
    exit 1
fi

--wVmyIxSo9dssAgWx
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=0001-Fix-potential-PANICs-with-concurrent-drop-of-pgstats.patch
Content-Transfer-Encoding: quoted-printable

=46rom 29c29dd0a778e952cdf6f55c7be08a33436515e2 Mon Sep 17 00:00:00 2001
=46rom: Michael Paquier <michael@paquier.xyz>
Date: Wed, 17 Jun 2026 12:59:40 +0900
Subject: [PATCH] Fix potential PANICs with concurrent drop of pgstats entri=
es

---
 src/include/utils/pgstat_internal.h             |  3 ++-
 src/backend/utils/activity/pgstat.c             |  2 +-
 src/backend/utils/activity/pgstat_function.c    |  2 +-
 src/backend/utils/activity/pgstat_replslot.c    |  2 +-
 src/backend/utils/activity/pgstat_shmem.c       | 17 ++++++++++++++++-
 src/backend/utils/activity/pgstat_xact.c        |  8 ++++----
 .../test_custom_stats/test_custom_var_stats.c   |  2 +-
 7 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat=
_internal.h
index fe463faaf639..3ca4f4548956 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -807,7 +807,8 @@ extern PgStat_EntryRef *pgstat_get_entry_ref(PgStat_Kin=
d kind, Oid dboid, uint64
 extern bool pgstat_lock_entry(PgStat_EntryRef *entry_ref, bool nowait);
 extern bool pgstat_lock_entry_shared(PgStat_EntryRef *entry_ref, bool nowa=
it);
 extern void pgstat_unlock_entry(PgStat_EntryRef *entry_ref);
-extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid);
+extern bool pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid,
+							  bool missing_ok);
 extern void pgstat_drop_all_entries(void);
 extern void pgstat_drop_matching_entries(bool (*do_drop) (PgStatShared_Has=
hEntry *, Datum),
 										 Datum match_data);
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activi=
ty/pgstat.c
index b67da88c7dc2..c4fa14f138fa 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -650,7 +650,7 @@ pgstat_shutdown_hook(int code, Datum arg)
 	dlist_init(&pgStatPending);
=20
 	/* drop the backend stats entry */
-	if (!pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber))
+	if (!pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber, fal=
se))
 		pgstat_request_entry_refs_gc();
=20
 	pgstat_detach_shmem();
diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/uti=
ls/activity/pgstat_function.c
index d47d05e3d922..f0366e139907 100644
--- a/src/backend/utils/activity/pgstat_function.c
+++ b/src/backend/utils/activity/pgstat_function.c
@@ -113,7 +113,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
 		if (!SearchSysCacheExists1(PROCOID, ObjectIdGetDatum(fcinfo->flinfo->fn_=
oid)))
 		{
 			pgstat_drop_entry(PGSTAT_KIND_FUNCTION, MyDatabaseId,
-							  fcinfo->flinfo->fn_oid);
+							  fcinfo->flinfo->fn_oid, true);
 			ereport(ERROR, errcode(ERRCODE_UNDEFINED_FUNCTION),
 					errmsg("function call to dropped function"));
 		}
diff --git a/src/backend/utils/activity/pgstat_replslot.c b/src/backend/uti=
ls/activity/pgstat_replslot.c
index 0d00dd5d93aa..a32b70a03736 100644
--- a/src/backend/utils/activity/pgstat_replslot.c
+++ b/src/backend/utils/activity/pgstat_replslot.c
@@ -188,7 +188,7 @@ pgstat_drop_replslot(ReplicationSlot *slot)
 	Assert(LWLockHeldByMeInMode(ReplicationSlotAllocationLock, LW_EXCLUSIVE));
=20
 	if (!pgstat_drop_entry(PGSTAT_KIND_REPLSLOT, InvalidOid,
-						   ReplicationSlotIndex(slot)))
+						   ReplicationSlotIndex(slot), false))
 		pgstat_request_entry_refs_gc();
 }
=20
diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/=
activity/pgstat_shmem.c
index b8f354c818a0..ae35332a3347 100644
--- a/src/backend/utils/activity/pgstat_shmem.c
+++ b/src/backend/utils/activity/pgstat_shmem.c
@@ -1000,13 +1000,16 @@ pgstat_drop_database_and_contents(Oid dboid)
  * This routine returns false if the stats entry of the dropped object cou=
ld
  * not be freed, true otherwise.
  *
+ * If missing_ok is true, skip entries that have been concurrently dropped.
+ *
  * The callers of this function should call pgstat_request_entry_refs_gc()
  * if the stats entry could not be freed, to ensure that this entry's memo=
ry
  * can be reclaimed later by a different backend calling
  * pgstat_gc_entry_refs().
  */
 bool
-pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid)
+pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint64 objid,
+				  bool missing_ok)
 {
 	PgStat_HashKey key =3D {0};
 	PgStatShared_HashEntry *shent;
@@ -1031,6 +1034,18 @@ pgstat_drop_entry(PgStat_Kind kind, Oid dboid, uint6=
4 objid)
 	shent =3D dshash_find(pgStatLocal.shared_hash, &key, true);
 	if (shent)
 	{
+		if (shent->dropped)
+		{
+			if (!missing_ok)
+				elog(ERROR,
+					 "trying to drop stats entry already dropped: kind=3D%s dboid=3D%u ob=
jid=3D%" PRIu64,
+					 pgstat_get_kind_info(shent->key.kind)->name,
+					 shent->key.dboid,
+					 shent->key.objid);
+			dshash_release_lock(pgStatLocal.shared_hash, shent);
+			return true;
+		}
+
 		freed =3D pgstat_drop_entry_internal(shent, NULL);
=20
 		/*
diff --git a/src/backend/utils/activity/pgstat_xact.c b/src/backend/utils/a=
ctivity/pgstat_xact.c
index 5e2d69e62979..3e1978775e16 100644
--- a/src/backend/utils/activity/pgstat_xact.c
+++ b/src/backend/utils/activity/pgstat_xact.c
@@ -85,7 +85,7 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_s=
tate, bool isCommit)
 			 * Transaction that dropped an object committed. Drop the stats
 			 * too.
 			 */
-			if (!pgstat_drop_entry(it->kind, it->dboid, objid))
+			if (!pgstat_drop_entry(it->kind, it->dboid, objid, true))
 				not_freed_count++;
 		}
 		else if (!isCommit && pending->is_create)
@@ -94,7 +94,7 @@ AtEOXact_PgStat_DroppedStats(PgStat_SubXactStatus *xact_s=
tate, bool isCommit)
 			 * Transaction that created an object aborted. Drop the stats
 			 * associated with the object.
 			 */
-			if (!pgstat_drop_entry(it->kind, it->dboid, objid))
+			if (!pgstat_drop_entry(it->kind, it->dboid, objid, true))
 				not_freed_count++;
 		}
=20
@@ -160,7 +160,7 @@ AtEOSubXact_PgStat_DroppedStats(PgStat_SubXactStatus *x=
act_state,
 			 * Subtransaction creating a new stats object aborted. Drop the
 			 * stats object.
 			 */
-			if (!pgstat_drop_entry(it->kind, it->dboid, objid))
+			if (!pgstat_drop_entry(it->kind, it->dboid, objid, true))
 				not_freed_count++;
 			pfree(pending);
 		}
@@ -323,7 +323,7 @@ pgstat_execute_transactional_drops(int ndrops, struct x=
l_xact_stats_item *items,
 		xl_xact_stats_item *it =3D &items[i];
 		uint64		objid =3D ((uint64) it->objid_hi) << 32 | it->objid_lo;
=20
-		if (!pgstat_drop_entry(it->kind, it->dboid, objid))
+		if (!pgstat_drop_entry(it->kind, it->dboid, objid, true))
 			not_freed_count++;
 	}
=20
diff --git a/src/test/modules/test_custom_stats/test_custom_var_stats.c b/s=
rc/test/modules/test_custom_stats/test_custom_var_stats.c
index 5c4871ed37ca..863d6a524925 100644
--- a/src/test/modules/test_custom_stats/test_custom_var_stats.c
+++ b/src/test/modules/test_custom_stats/test_custom_var_stats.c
@@ -600,7 +600,7 @@ test_custom_stats_var_drop(PG_FUNCTION_ARGS)
=20
 	/* Drop entry and request GC if the entry could not be freed */
 	if (!pgstat_drop_entry(PGSTAT_KIND_TEST_CUSTOM_VAR_STATS, InvalidOid,
-						   PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name)))
+						   PGSTAT_CUSTOM_VAR_STATS_IDX(stat_name), false))
 		pgstat_request_entry_refs_gc();
=20
 	PG_RETURN_VOID();
--=20
2.54.0


--wVmyIxSo9dssAgWx--

--j9H3Lna40EX9MUWT
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmoyH1EACgkQnvQgOdby
QH1gxhAAgl5D9U7A/XuueqiqbJx+p+Fo22RZFK9aj3qHBewalGLcLDHs0Rw2Tqer
MEYI6mysWg6zFzvOjAbYttiZj8aGbctaXXk1tEVOAzyJZQiQ1t1Bd7M4vfxJZjJi
Iw3VztERW0tV9z3pI3CHJ+K57bWuUeAkvq39MoOYOOSOK8ZTjnNANrmyZzthBFt2
wznpsz/A60EjkwIABT4y7Af+YfrScjb0kjR95EFjdanzJ2w4pkYGX9ykdQSd/yXM
XZ0lnRMKjAcDJyyu/eQklSA52XOvQkXhMZxAdueF/mU3yO14hIeeRyaYABELojQC
Og4jH/M3jHPzmpR3F9G1WlRb//Ai0oFph/HkcLGIadO7fAEkiSJEa2/sPWsWIVD/
Wl7sq2TmfxJvPd/fq44BE7x6+ETs+8ZKfIq8qoadeq5bqeCaXckd+wEIruVeooWW
ryWROJNMIbgco0duTeLngGK1Lm+rvOTT8AV+M8FOUpiQpBcwcAnSiWfSwbD8eWwc
6xlDBgv1RMlAaKihQAwM5MN7ueTWbcfMBPigBCLgrI1wBMML7Xr7OZOaWbrmRa3L
0nZLCju2FNm+1/F1McZ/w2K3YzJd9KThF4k7SZhKAAaSxnUBf2LhK7yHYcKmvFGc
Nvdal/9YWgy/W7M0xiIgosoR3kZRjB0xHhB/SbxVOvi1U+qejoM=
=0p/W
-----END PGP SIGNATURE-----

--j9H3Lna40EX9MUWT--