Re: BUG #17122: panic on prepare with subsequent pg_advisory_lock() and pg_advisory_xact_lock_shared()

I wrote:
> PG Bug reporting form <noreply(at)postgresql(dot)org> writes:
>> The following statements lead to panic:
>> begin;
>> select pg_advisory_lock(1);
>> select pg_advisory_xact_lock_shared(1);
>> prepare transaction 'test';

> Thanks for the report! Looks like the shared lock is sufficient
> to cause the problem:

Ah, scratch that. I can't reproduce it now, and I think I messed
up yesterday by not remembering that pg_advisory_lock() takes a
session-level lock that would continue to be held after PREPARE.

> I also tried this:
> ...
> regression=*# prepare transaction 'test';
> ERROR: cannot PREPARE while holding both session-level and transaction-level locks on the same object
> which makes me wonder why we didn't issue that error in your
> example case.

The reason why not is that the code that's meant to detect that is
just fundamentally inadequate. It's examining individual LOCALLOCK
entries to detect conflicts, but we keep separate LOCALLOCK entries
for different lockmodes on the same object. So pg_advisory_lock()
creates an entry with lockmode ExclusiveLock, while
pg_advisory_xact_lock_shared() creates a different entry with lockmode
AccessShareLock. But they are pointing at the same PROCLOCK, and the
restriction we're dealing with here applies at the PROCLOCK level.

So I think we need something like the attached to fix this.

In the longer term, maybe it'd be better to rethink how we represent
LOCALLOCK entries so that they are one-to-one with PROCLOCKs. But
rearranging that data structure for the convenience of PREPARE
TRANSACTION is probably going to be a net loss performance-wise.
In any case, we certainly wouldn't risk back-patching such a change.

regards, tom lane