diff options
| author | Peter Geoghegan <pg@bowt.ie> | 2020-07-21 15:50:58 -0700 |
|---|---|---|
| committer | Peter Geoghegan <pg@bowt.ie> | 2020-07-21 15:50:58 -0700 |
| commit | 4a70f829d86cb8dbd68f561720e6329f5e818c94 (patch) | |
| tree | 50c2720e1591c314ef145bc6ab4d81881f2c053f /src/include | |
| parent | 670c0a1d474bf296dbcc1d6de912d4841f2ed643 (diff) | |
| download | postgresql-4a70f829d86cb8dbd68f561720e6329f5e818c94.tar.gz postgresql-4a70f829d86cb8dbd68f561720e6329f5e818c94.zip | |
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
Diffstat (limited to 'src/include')
| -rw-r--r-- | src/include/access/nbtree.h | 4 | ||||
| -rw-r--r-- | src/include/pg_config_manual.h | 8 |
2 files changed, 9 insertions, 3 deletions
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h index 79506c748b2..f5274cc7508 100644 --- a/src/include/access/nbtree.h +++ b/src/include/access/nbtree.h @@ -1073,6 +1073,10 @@ extern Buffer _bt_getbuf(Relation rel, BlockNumber blkno, int access); extern Buffer _bt_relandgetbuf(Relation rel, Buffer obuf, BlockNumber blkno, int access); extern void _bt_relbuf(Relation rel, Buffer buf); +extern void _bt_lockbuf(Relation rel, Buffer buf, int access); +extern void _bt_unlockbuf(Relation rel, Buffer buf); +extern bool _bt_conditionallockbuf(Relation rel, Buffer buf); +extern void _bt_upgradelockbufcleanup(Relation rel, Buffer buf); extern void _bt_pageinit(Page page, Size size); extern bool _bt_page_recyclable(Page page); extern void _bt_delitems_vacuum(Relation rel, Buffer buf, diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h index 45b6a457896..705dc69c06a 100644 --- a/src/include/pg_config_manual.h +++ b/src/include/pg_config_manual.h @@ -271,11 +271,13 @@ * Valgrind understands PostgreSQL memory contexts. This permits detecting * memory errors that Valgrind would not detect on a vanilla build. It also * enables detection of buffer accesses that take place without holding a - * buffer pin. See also src/tools/valgrind.supp. + * buffer pin (or without holding a buffer lock in the case of index access + * methods that superimpose their own custom client requests on top of the + * generic bufmgr.c requests). See also src/tools/valgrind.supp. * * "make installcheck" is significantly slower under Valgrind. The client - * requests fall in hot code paths, so USE_VALGRIND slows native execution by - * a few percentage points even when not run under Valgrind. + * requests fall in hot code paths, so USE_VALGRIND slows execution by a few + * percentage points even when not run under Valgrind. * * You should normally use MEMORY_CONTEXT_CHECKING with USE_VALGRIND; * instrumentation of repalloc() is inferior without it. |