Use OOM-safe routine for pgstats shared hashtable insert

pgstat_get_entry_ref() called dshash_find_or_insert() to insert a new
entry into the shared pgstats hashtable.  If the allocation required for
the new entry fails, a local cache entry would be left in
pgStatEntryRefHash with a NULL shared_entry.  This code is switched to
use dshash_find_or_insert_extended(), so as local hash entries can be
cleaned up before failuring.  This change makes pgstat_get_entry_ref()
more robust on re-entry.

Taken in isolation within a single backend, local entries are handled
fine on HEAD as pgstat_get_entry_ref() is able to handle the case of a
NULL shared_entry on re-entry.  However it is possible to finish with
some NULL pointer dereference in more complex scenarios, like:
- First backend fails insert into the shared hashtable on OOM.
- Second backend bumps the same entry's refcount (due to
reinitialization of the same entry, for example).
- First backend calls pgstat_gc_entry_refs(), uses its local entry
reference with the NULL shared_entry, crashes when checking if the
shared entry has been dropped.

6f0738ddec85 has introduced dshash_find_or_insert_extended() for the
same reason as what we are dealing with here: the backend could be left
in an inconsistent stat, and clean up actions need to be taken before
issuing an error.  The failure is unlikely going to show up in practice,
so no backpatch is done.  This hash insert extended routine is new as of
v19.

Author: Michael Paquier <[email protected]>
Discussion: https://postgr.es/m/[email protected]

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/127ce776983f19e81ba8abda1984f6f0e229c1b5

Modified Files
--------------
src/backend/utils/activity/pgstat_shmem.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

Reply via email to