Re: patch: fix race in SSI's CheckTargetForConflictsIn - Mailing list pgsql-hackers

From Dan Ports
Subject Re: patch: fix race in SSI's CheckTargetForConflictsIn
Date
Msg-id 20110507024922.GA53228@csail.mit.edu
Whole thread Raw
In response to Re: patch: fix race in SSI's CheckTargetForConflictsIn  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: patch: fix race in SSI's CheckTargetForConflictsIn  (Dan Ports <drkp@csail.mit.edu>)
List pgsql-hackers
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/


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Backpatching of "Teach the regular expression functions to do case-insensitive matching"
Next
From: Shiv
Date:
Subject: Re: improvements to pgtune