Alvaro Herrera <[EMAIL PROTECTED]> writes:
> Here is a small patch to refactor common functionality out of
> LockRelease and LockReleaseAll, creating a new static function
> RemoveProcLock.
> (This comes from Heikki's two-phase commit patch, where it is used more.)

I was just looking at this code in the context of Heikki's patch, and
it seemed to have some issues: specifically that the code
                if (wakeupNeeded)
                        ProcLockWakeup(lockMethodTable, lock);
was formerly run only if the lock still had nRequested > 0.  Since the
case where nRequested == 0 causes the lock to be physically removed,
it would not be merely inefficient but actually a use of a dangling
pointer to call ProcLockWakeup when the lock's been removed.  However
the patched code now does the above unconditionally.  Can you prove
that wakeupNeeded will never be true when nRequested == 0?

It might be safer to migrate the ProcLockWakeup call inside

FWIW, I agree with turning the WARNINGs into ERRORs and removing the
useless return value from LockRelease et al.

                        regards, tom lane

---------------------------(end of broadcast)---------------------------
TIP 8: explain analyze is your friend

Reply via email to