Race condition between PREPARE TRANSACTION and COMMIT PREPARED (was Re: Problem with txid_snapshot_in/out() functionality)

On 04/12/2014 05:03 PM, Andres Freund wrote:
>> >If we don't, aren't we letting other backends see non-self-consistent
>> >state in regards to who holds which locks, for example?
> I think that actually works out ok, because the locks aren't owned by
> xids/xacts, but procs. Otherwise we'd be in deep trouble in
> CommitTransaction() as well where ProcArrayEndTransaction() clearing
> that state.
> After the whole xid transfer, there's PostPrepare_Locks() transferring
> the locks.

Right.

However, I just noticed that there's a race condition between PREPARE 
TRANSACTION and COMMIT/ROLLBACK PREPARED. PostPrepare_Locks runs after 
the prepared transaction is already marked as fully prepared. That means 
that by the time we get to PostPrepare_Locks, another backend might 
already have finished and removed the prepared transaction. That leads 
to a PANIC (put a breakpoint just before PostPrepare_Locks):

postgres=# commit prepared 'foo';
PANIC:  failed to re-find shared proclock object
PANIC:  failed to re-find shared proclock object
The connection to the server was lost. Attempting reset: Failed.

FinishPrepareTransaction reads the list of locks from the two-phase 
state file, but PANICs when it doesn't find the corresponding locks in 
the lock manager (because PostPrepare_Locks hasn't transfered them to 
the dummy PGPROC yet).

I think we'll need to transfer of the locks earlier, before the 
transaction is marked as fully prepared. I'll take a closer look at this 
tomorrow.

- Heikki