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

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jan Wieck <jan(at)wi3ck(dot)info>, PostgreSQL Development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Race condition between PREPARE TRANSACTION and COMMIT PREPARED (was Re: Problem with txid_snapshot_in/out() functionality)
Date: 2014-05-15 14:38:21
Message-ID: 20140515143821.GE23662@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2014-05-15 17:21:28 +0300, Heikki Linnakangas wrote:
> >Is it guaranteed that all paths have called LWLockReleaseAll()
> >before calling the proc exit hooks? Otherwise we might end up waiting
> >for ourselves...
>
> Hmm. AbortTransaction() will release locks before we get here, but the
> before_shmem_exit() callpath will not. So an elog(FATAL), while holding
> TwoPhaseStateLock would cause us to deadlock with ourself. But there are no
> such elogs.

> I copied this design from async.c, which is quite similar, so if there's a
> problem that ought to be fixed too. And there are other more complicated
> before_shmem callbacks that worry me more, like createdb_failure_callback().
> But I think they're all all right.

Perhaps we should enforce that LWLockReleaseAll() is called first?
E.g. in shmem_exit()? It'll happen in ProcKill() atm, but that's
normally pretty much at the bottom of the stack.

> >It's not particularly nice to register shmem exit hooks in the middle of
> >normal processing because it makes it impossible to use
> >cancel_before_shmem_exit() previously registered hooks. I think this
> >should be registered at startup, if max_prepared_xacts > 0.
>
> <shrug>. async.c and namespace.c does the same, and it hasn't been a
> problem.

Well, it doesn't seem unreasonable to have C code using
PG_ENSURE_ERROR_CLEANUP/PG_END_ENSURE_ERROR_CLEANUP around a 2pc commit
to me. That'll break with this.
Perhaps we should just finally make cancel_before_shmem_exit search the
stack of callbacks.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-05-15 14:49:38 Unportability of setvbuf()
Previous Message Amit Langote 2014-05-15 14:36:51 Re: proposal: Set effective_cache_size to greater of .conf value, shared_buffers