Re: Race condition in backend process exit

On Sun, Aug 07, 2005 at 03:45:10PM -0400, Tom Lane wrote:

> I'm thinking that the correct fix is to forget the notion that it's
> safer to do a subset of AbortTransaction than to do the whole thing.
> We should make ShutdownPostgres do this:
>
> AbortOutOfAnyTransaction();
>
> /* Drop user-level locks, which are not dropped by xact abort */
> #ifdef USER_LOCKS
> LockReleaseAll(USER_LOCKMETHOD, true);
> #endif
>
> and then remove the lock manager cleanup operations from ProcKill.

I agree it's cleaner. It'd be comforting however if any cleanup
procedure would severely report when it finds inconsistent state (Most
of xact.c throws at least a WARNING, IIRC). That way we'd know about
bogus conditions quickly. OTOH, that code is in much better shape than
it was when ShutdownPostgres was last heavily modified, which AFAICS was
around revision 1.82.

> I'm a bit hesitant to back-patch such a nontrivial and hard-to-test
> change, but it sure looks badly broken to me. Any thoughts about the
> risks involved?

How far back? I'd do it in 8.0 but not in earlier releases. The
transaction management code changed a lot in between, and I think a lot
of bugs were corrected.

--
Alvaro Herrera (<alvherre[a]alvh.no-ip.org>)
Jason Tesser: You might not have understood me or I am not understanding you.
Paul Thomas: It feels like we're 2 people divided by a common language...