Proposed patch for LISTEN/NOTIFY race condition - Mailing list pgsql-patches

From Tom Lane
Subject Proposed patch for LISTEN/NOTIFY race condition
Date
Msg-id 8626.1205277664@sss.pgh.pa.us
Whole thread Raw
Responses Re: Proposed patch for LISTEN/NOTIFY race condition  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Proposed patch for LISTEN/NOTIFY race condition  (Alvaro Herrera <alvherre@commandprompt.com>)
List pgsql-patches
This patch responds to the problem reported by Laurent Birtz here:
http://archives.postgresql.org/pgsql-bugs/2008-03/msg00094.php
The difficulty is that LISTEN modifies pg_listener and then releases
lock early, so that there is an interval where a concurrent NOTIFY
doesn't see the pg_listener entry.  In simple cases this causes no
problem, but given just the right interleaving of execution it is
possible for a listener to miss a notification that it "should have"
gotten.  The proposed fix moves all updating of pg_listener to end
of transaction, making it reasonable to hold the exclusive lock on
pg_listener until commit.  (The simpler fix of just not releasing
lock during LISTEN seems to me too prone to result in deadlocks,
since applications might try to touch random other tables in that
same transaction.)

The patch is fairly bulky but it is actually an extremely
straightforward refactoring of the existing code, plus a nearly exact
copy-and-paste of the existing logic that handles the pending-NOTIFY
list to construct code that handles a pending-LISTEN-and-UNLISTEN list.
I had thought there would be some complexity in managing the
pending-actions list, but as long as we don't try to be smart about
folding out redundant or conflicting commands, it really is trivial.
We just apply the same sequence of table updates that we would have
anyway, only a bit later in the transaction.

I took the opportunity to clean up one or two bits of ancient crufty
code, notably changing some volatile interrupt-service flag variables
from "int" to "sig_atomic_t" and using proper GetDatum macros instead
of casts.

The patch disallows LISTEN/NOTIFY in a prepared transaction, and there
is also a side effect on whether a transaction can see entries in
pg_listener for its own uncommitted LISTEN commands.  I think fixing
the race condition is sufficient justification for these potential small
incompatibilities ... anyone think differently?

This patch will apply against HEAD and 8.3, but not cleanly against
prior versions.  Since this code hasn't changed materially (except for
additions of subtransaction and 2PC support) for a long time,
back-patching should be straightforward, but I haven't actually done
that yet.

Comments?

            regards, tom lane


Attachment

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: SPI-header-files safe for C++-compiler
Next
From: Bruce Momjian
Date:
Subject: Re: [BUGS] psql \COPY accepts multiple NULL AS