diff --git a/contrib/test_decoding/t/001_repl_stats.pl b/contrib/test_decoding/t/001_repl_stats.pl index c335bc5890c..33a84bb5e08 100644 --- a/contrib/test_decoding/t/001_repl_stats.pl +++ b/contrib/test_decoding/t/001_repl_stats.pl @@ -178,4 +178,42 @@ ok(-f "$stats_file", "stats file must exist after shutdown"); $bpgsql->quit; +# Session holding a stale reference to a slot stats entry, reinitialized +# then dropped again. It must be freed on exit. +$node->start; + +my $slot_name_stale = 'regression_slot6'; +$node->safe_psql('postgres', + "SELECT pg_create_logical_replication_slot('$slot_name_stale', 'test_decoding');" +); + +$bpgsql = $node->background_psql('postgres', on_error_stop => 1); +$bpgsql->query_safe( + "SELECT pg_logical_slot_peek_binary_changes('$slot_name_stale', NULL, NULL)" +); + +# Entry dropped, still kept by the session with a reference. +$node->safe_psql('postgres', + "SELECT pg_drop_replication_slot('$slot_name_stale')"); + +# Entry reinitialized for the new slot, then dropped again. The session +# still holds its reference. +$node->safe_psql( + 'postgres', qq[ + SELECT pg_create_logical_replication_slot('regression_slot7', 'test_decoding'); + SELECT pg_drop_replication_slot('regression_slot7'); +]); + +# This session is the last referrer, and frees the entry on exit. +$bpgsql->quit; + +$node->stop; + +command_like( + [ 'pg_controldata', $node->data_dir ], + qr/Database cluster state:\s+shut down\n/, + 'node shut down ok with stale stats reference'); +ok(-f "$stats_file", + "stats file must exist after shutdown with stale stats reference"); + done_testing(); diff --git a/src/backend/utils/activity/pgstat_shmem.c b/src/backend/utils/activity/pgstat_shmem.c index b8e5b8134e2..603bda1cc02 100644 --- a/src/backend/utils/activity/pgstat_shmem.c +++ b/src/backend/utils/activity/pgstat_shmem.c @@ -601,39 +601,25 @@ pgstat_release_entry_ref(PgStat_HashKey key, PgStat_EntryRef *entry_ref, PgStatShared_HashEntry *shent; /* - * We're the last referrer to this entry, try to drop the shared - * entry. + * We're the last referrer. Before we get the lock, the entry may + * have been reinitialized, or freed and created again, so look it + * up by key. If it is gone, it got already dropped and freed. */ + shent = dshash_find(pgStatLocal.shared_hash, &key, true); - /* only dropped entries can reach a 0 refcount */ - Assert(entry_ref->shared_entry->dropped); - - shent = dshash_find(pgStatLocal.shared_hash, - &entry_ref->shared_entry->key, - true); - if (!shent) - elog(ERROR, "could not find just referenced shared stats entry"); - - /* - * This entry may have been reinitialized while trying to release - * it, so double-check that it has not been reused while holding a - * lock on its shared entry. - */ - if (pg_atomic_read_u32(&entry_ref->shared_entry->generation) == - entry_ref->generation) + if (shent) { - /* Same "generation", so we're OK with the removal */ - Assert(pg_atomic_read_u32(&entry_ref->shared_entry->refcount) == 0); - Assert(entry_ref->shared_entry == shent); - pgstat_free_entry(shent, NULL); - } - else - { - /* - * Shared stats entry has been reinitialized, so do not drop - * its shared entry, only release its lock. - */ - dshash_release_lock(pgStatLocal.shared_hash, shent); + if (shent->dropped && + pg_atomic_read_u32(&shent->refcount) == 0) + { + /* Entry dropped and unreferenced, so free it. */ + pgstat_free_entry(shent, NULL); + } + else + { + /* Entry reinitialized or created again, still in use. */ + dshash_release_lock(pgStatLocal.shared_hash, shent); + } } } }