Re: patch: fix race in SSI's CheckTargetForConflictsIn

On Fri, May 06, 2011 at 09:35:39PM -0400, Robert Haas wrote:
> Why does this HASH_FIND the applicable hash table entries and then
> HASH_REMOVE it as a separate step, instead of just HASH_REMOVE-ing it
> in one go?

For PredicateLockHash, we need to find the lock entry first so that we
can call SHMQueueDelete on its targetLink and xactLink fields. 

For LocalPredicateLockHash, we check the resulting entry to decide
whether to remove it. Having looked at the code some more, however, we do
always remove it because we only apply this optimization to heap tuple
locks, which are not parents of other locks. So we can simplify this
down to a HASH_REMOVE.


> Doesn't this fail to release the locks if rmpredlock == NULL?

Yikes. Indeed it does.

> I believe it's project style to test (rmpredlock != NULL) rather than
> just (rmpredlock).

That works for me (I prefer the != NULL myself). I believe I've seen
both elsewhere, though...

Will update the patch.

Dan

-- 
Dan R. K. Ports              MIT CSAIL                http://drkp.net/