pgsql: Add nbtree Valgrind buffer lock checks. - Mailing list pgsql-committers

From Peter Geoghegan
Subject pgsql: Add nbtree Valgrind buffer lock checks.
Date
Msg-id E1jy16s-0004ga-9K@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
Add nbtree Valgrind buffer lock checks.

Holding just a buffer pin (with no buffer lock) on an nbtree buffer/page
provides very weak guarantees, especially compared to heapam, where it's
often safe to read a page while only holding a buffer pin.  This commit
has Valgrind enforce the following rule: it is never okay to access an
nbtree buffer without holding both a pin and a lock on the buffer.

A draft version of this patch detected questionable code that was
cleaned up by commits fa7ff642 and 7154aa16.  The code in question used
to access an nbtree buffer page's special/opaque area with no buffer
lock (only a buffer pin).  This practice (which isn't obviously unsafe)
is hereby formally disallowed in nbtree.  There doesn't seem to be any
reason to allow it, and banning it keeps things simple for Valgrind.

The new checks are implemented by adding custom nbtree client requests
(located in LockBuffer() wrapper functions); these requests are
"superimposed" on top of the generic bufmgr.c Valgrind client requests
added by commit 1e0dfd16.  No custom resource management cleanup code is
needed to undo the effects of marking buffers as non-accessible under
this scheme.

Author: Peter Geoghegan
Reviewed-By: Anastasia Lubennikova, Georgios Kokolatos
Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/4a70f829d86cb8dbd68f561720e6329f5e818c94

Modified Files
--------------
src/backend/access/nbtree/nbtinsert.c |   2 +-
src/backend/access/nbtree/nbtpage.c   | 145 +++++++++++++++++++++++++++++-----
src/backend/access/nbtree/nbtree.c    |   5 +-
src/backend/access/nbtree/nbtsearch.c |  23 +++---
src/backend/access/nbtree/nbtutils.c  |   4 +-
src/backend/storage/buffer/bufmgr.c   |  23 +++++-
src/include/access/nbtree.h           |   4 +
src/include/pg_config_manual.h        |   8 +-
8 files changed, 170 insertions(+), 44 deletions(-)


pgsql-committers by date:

Previous
From: Tom Lane
Date:
Subject: pgsql: Weaken type-OID-matching checks in array_recv and record_recv.
Next
From: Tom Lane
Date:
Subject: pgsql: neqjoinsel must now pass through collation to eqjoinsel.