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 RemoveProcLock. 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