rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix minor nbtree page deletion buffer lock issue. 

Avoid accessing the deletion target page's special area during nbtree
page deletion at a point where there is no buffer lock held.  This issue
was detected by a patch that extends Valgrind's memcheck tool to mark
nbtree pages that are unsafe to access (due to not having a buffer lock
or buffer pin) as NOACCESS.

We do hold a buffer pin at this point, and only access the special area,
so the old approach was safe.  Even still, it seems like a good idea to
tighten up the rules in this area.  There is no reason to not simply
insist on always holding a buffer lock (not just a pin) when accessing
nbtree pages.

Update some related comments in passing.

Discussion: https://postgr.es/m/CAH2-WzkLgyN3zBvRZ1pkNJThC=xi_0gpWRUb_45eexLH1+k2_Q@mail.gmail.com
This commit is contained in:
Peter Geoghegan 2020-04-25 14:17:02 -07:00
parent f246ea3b2a
commit fa7ff642c2
1 changed files with 9 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/backend/access/nbtree/nbtpage.c
View File

@ -1611,7 +1611,7 @@ _bt_pagedel(Relation rel, Buffer buf)
* to-be-deleted doesn't have a downlink, and the page
* deletion algorithm isn't prepared to handle that.
*/
if (!P_LEFTMOST(opaque))
if (leftsib != P_NONE)
{
BTPageOpaque lopaque;
Page lpage;
@ -1695,6 +1695,12 @@ _bt_pagedel(Relation rel, Buffer buf)
* the only child of the parent, and could be removed. It would be
* picked up by the next vacuum anyway, but might as well try to
* remove it now, so loop back to process the right sibling.
*
* Note: This relies on the assumption that _bt_getstackbuf() will be
* able to reuse our original descent stack with a different child
* block (provided that the child block is to the right of the
* original leaf page reached by _bt_search()). It will even update
* the descent stack each time we loop around, avoiding repeated work.
*/
if (!rightsib_empty)
break;
@ -1904,8 +1910,8 @@ _bt_mark_page_halfdead(Relation rel, Buffer leafbuf, BTStack stack)
* leaf page is deleted.
*
* Returns 'false' if the page could not be unlinked (shouldn't happen).
* If the (new) right sibling of the page is empty, *rightsib_empty is set
* to true.
* If the (current) right sibling of the page is empty, *rightsib_empty is
* set to true.
*
* Must hold pin and lock on leafbuf at entry (read or write doesn't matter).
* On success exit, we'll be holding pin and write lock. On failure exit,