From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Geoghegan <pg@bowt.ie>
cc: Andres Freund <andres@anarazel.de>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: PANIC: wrong buffer passed to visibilitymap_clear
In-reply-to: 
 <CAH2-WzkCXg6RQJdc_7ZGHcKvyoJAfFu9R2ZVAiE9mCcOfaXJdQ@mail.gmail.com>
References: <2247102.1618008027@sss.pgh.pa.us>
 <20210409232739.sazp5p4xnnagrusa@alap3.anarazel.de>
 <20210409233203.3khg3s5iw34g4xrc@alap3.anarazel.de>
 <2644575.1618117467@sss.pgh.pa.us>
 <CAH2-WzmWOLaWhQgEC1BJN0JZpOCOwxCAhjmd2Nd4yh4wrqju8Q@mail.gmail.com>
 <2679216.1618156670@sss.pgh.pa.us>
 <CAH2-WzmJ7THDU3jPgRH8J++b+pSoOh8nfGZEhNHh+4=LFatDeg@mail.gmail.com>
 <CAH2-WzkCXg6RQJdc_7ZGHcKvyoJAfFu9R2ZVAiE9mCcOfaXJdQ@mail.gmail.com>
Comments: In-reply-to Peter Geoghegan <pg@bowt.ie>
	message dated "Sun, 11 Apr 2021 09:43:15 -0700"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2689872.1618161194.1@sss.pgh.pa.us>
Date: Sun, 11 Apr 2021 13:13:14 -0400
Message-ID: <2689873.1618161194@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2689873.1618161194%40sss.pgh.pa.us>
Precedence: bulk

Peter Geoghegan <pg@bowt.ie> writes:
> This isn't just any super-exclusive lock, either -- we were calling
> ConditionalLockBufferForCleanup() at this point.

> I now think that there is a good chance that we are seeing these
> symptoms because the "conditional-ness" went away -- we accidentally
> relied on that.

Ah, I see it.  In the fragment of heap_update where we have to do some
TOAST work (starting at line 3815) we transiently *release our lock*
on the old tuple's page.  Unlike the earlier fragments that did that,
this code path has no provision for rechecking whether the page has
become all-visible, so if that does happen while we're without the
lock then we lose.  (It does look like RelationGetBufferForTuple
knows about updating vmbuffer, but there's one code path through the
if-nest at 3850ff that doesn't call that.)

So the previous coding in vacuumlazy didn't tickle this because it would
only set the all-visible bit on a page it had superexclusive lock on;
that is, continuing to hold the pin was sufficient.  Nonetheless, if
four out of five paths through heap_update take care of this matter,
I'd say it's heap_update's bug not vacuumlazy's bug that the fifth path
doesn't.

			regards, tom lane