2pc gxact lsn race

Hi!

While playing around with two-phase transactions, I've noticed something
strange. The prepare_start_lsn and prepare_end_lsn fields in gxact are
modified within CheckPointTwoPhase() without the use of proper locking,
which means they could become inconsistent.

The original commit by Tom (e26b0abda391, from 2005) only applied a
SHARED lock. Later, Simon Riggs added commit 728bd991c3c4, "Speedup 2PC
recovery," which moved the invalidating of these fields from the
MarkAsPreparing() to CheckPointTwoPhase(), but no locking was
implemented in this new location.

I suspected that this issue would be easy to reproduce using injection
points, and that turned out to be the case. There are three problematic
places: the calls to FinishPreparedTransaction(), EndPrepare(), and
LookupGXact(). In one instance, running the tests triggers an assertion
failure; in others, the logic governing two-phase transactions is
compromised. I totally understand that this is a *very* rare scenario
in real-world use, but it is still something worth being aware of.

As for a fix, I’ve implemented some ad-hoc solutions, PFA patch set.
Actually, I don't really understand why the invalidation of the
prepare_start_lsn and prepare_end_lsn fields is required within
CheckPointTwoPhase(). Is it possible to get rid of them entirely without
breaking something?

Thoughts?

--
Best regards,
Maxim Orlov.