[svk-devel] All Commands releasing giant_lock to early

[Bernhard Weisshuhn]

OK, let's hope I don't make a fool out of my self with this one.
I've been pulling my hair since ours on this:

History:
Last year I reported a problem related to locking when using more than
one SVK::Commands in one session. Original post with testcase is here:

http://lists.bestpractical.com/pipermail/svk-devel/2006-October/000195.html

Then something (didn't check back then) got fixed and my testcase
worked again, at least that is what I was writing then (don't really
remember):

http://lists.bestpractical.com/pipermail/svk-devel/2006-October/000219.html

Now, with 2.0 things are broken again.


I narrowed it down to what I suspect to be a bug in Command.pm, method
run_command, line 227-228, just after the "nasty" eval for parse_arg:

    
    # in case parse_arg dies, unlock giant
    $self->{xd}->giant_unlock if $self->{xd} && ref ($self) && !$self->{hold_giant};
    die $@ if $@;
        return $ret;

This leeds to _every_ command releasing the giant lock. And this leads to
a checkout not beeing locked (giant lock is gone, XD::store tries to
merge with locked directories, which never made it there). Subsequent
merging of changes in the XD object fail silently.

I think a check for $@ should be added to the condition of the
giant_unlock(), so that the code does what the comment says, namely
only release the lock if the eval failed, like so:
    
    $self->{xd}->giant_unlock if $@ && $self->{xd} && ref ($self) && !$self->{hold_giant};

At least my code works after I made that change. 

What do you guys think? The locking stuff is a bit hairy, it is late and
I've been staring at the screen for too long...

Any feedback appreciated.

regards,
  bkw