SSI can miss conflicts between index-only scans and heap writes

Hi,

I found a race between index-only scans and heap_delete()/heap_update()
that lets two SERIALIZABLE transactions both commit even though their
reads are inconsistent with any serial execution order. I reproduced
it on master at 374522aa63a.

heap_delete() and heap_update() check for conflicts before changing the
tuple and clearing the page's visibility-map bit. Their comments assume
that holding the heap buffer lock excludes readers until the change is
visible. An index-only scan can still read the old index entry while
the VM bit is set, without taking that buffer lock.

Here is the DELETE case. Each table has one row, and both have been
vacuumed so that the reader can use an index-only scan:

CREATE TABLE ios_a (id int PRIMARY KEY);
CREATE TABLE ios_b (id int, present bool DEFAULT true);
CREATE INDEX ios_b_idx ON ios_b (id) WHERE present;
INSERT INTO ios_a VALUES (1);
INSERT INTO ios_b VALUES (1);
VACUUM (FREEZE, ANALYZE) ios_a;
VACUUM (FREEZE, ANALYZE) ios_b;

Both sessions start with BEGIN ISOLATION LEVEL SERIALIZABLE, with
enable_seqscan and enable_bitmapscan off. Using GDB, I paused s2 in
heap_delete() just after CheckForSerializableConflictIn() returned,
before clearing the VM bit:

s2: SELECT id FROM ios_a; -- returns 1
s2: DELETE FROM ios_b; -- paused
s1: SELECT id FROM ios_b WHERE present; -- returns 1 (index-only)
s1: DELETE FROM ios_a;
s1: COMMIT;
-- Resume s2.
s2: COMMIT; -- also succeeds

Neither serial order produces these reads: running s1 first would make
s2's SELECT return no rows, and running s2 first would make s1's SELECT
return no rows.

There is also a reader-side window. On fresh copies of the tables, I
paused s1 in heapam_index_getnext_slot(), after VM_ALL_VISIBLE returned
true but before PredicateLockPage():

s2: SELECT id FROM ios_a; -- returns 1
s1: SELECT id FROM ios_b WHERE present; -- paused
s2: DELETE FROM ios_b;
s2: COMMIT;
-- Resume s1: its SELECT returns 1 without checking the heap.
s1: DELETE FROM ios_a;
s1: COMMIT; -- also succeeds

Replacing s2's DELETE with UPDATE ios_b SET present = false reproduces
both cases too. The new row no longer satisfies ios_b_idx's WHERE
present condition, so no new index entry is inserted. This avoids an
index insertion's conflict check hiding the heap race.

The attached patch adds a second conflict-in check to heap_delete() and
heap_update() after clearing an all-visible bit, outside the critical
section and before releasing the heap buffer lock. In
heapam_index_getnext_slot(), a SERIALIZABLE index-only scan rechecks the
VM bit after PredicateLockPage() and fetches the heap if it was cleared.
The writer rechecks for predicate locks acquired since its first check.
The reader rechecks for a VM bit cleared before it acquired its
predicate lock. Both changes are needed.

The patch includes injection-point tests for both orderings, with DELETE
and UPDATE. With the fix, all four cases abort one transaction with
40001. The same schedules still allow both transactions to commit under
READ COMMITTED and REPEATABLE READ.

make check, the core isolation suite, and injection_points tests pass.

Regards,
Rui