lock-breaking ui

John A Meinel john at arbash-meinel.com
Mon Feb 20 15:07:16 GMT 2006


Martin Pool wrote:
> On Thu, 2006-02-16 at 20:29 -0600, John A Meinel wrote:
>> Robert Collins wrote:

One bug I just conceptually found, how do you determine that you own the
lock before you unlock it, without having a race condition? Details below.
...

> There are several options
> 
>  0: leave them as is, using protocol-specific locking
> 
>  1: change them to use LockDirs instead - will give protection between
> new clients using old branches
> 
>  2: use both LockDirs and protocol-specific methods: will give
> transport-specific protection for old clients, and better protection for
> new clients
> 
>  3: remove them entirely, so new clients on old branches have no
> protection
> 
> Of these I prefer 2, and then 0.

I probably prefer doing double-protocol specific locking (so always
create branch-lock.write-lock, even locally, and OS level lock locally),
and not adding LockDirs, because that gives 2 locks which need to be broken.

Also, I would like to get away from older formats as much as possible.
I'm okay with your (2), but I prefer an extended 0.

...

>> By the way, have we worked out any of the ui for breaking locks?
> 
> Robert and I talked a little, and thought this:
> 
> * users or shell scripts should never need to take fine-grained locks
> (the shell interface isn't so fine-grained as to make it useful), so all
> they need is to display or break locks

I definitely agree with this.

> 
>  * it should be possible to show the descriptive information of a lock,
> or to break it

Yes.

> 
>  * when interactively breaking the lock the user should probably be
> shown the descriptive information first.  there's a possible race here
> when someone else acquires the lock between the user seeing it and
> deciding to break it

If we make it an interactive command, definitely. So you would have "bzr
break-lock" display the lock information, and ask the user if they are sure.

Though the only interactive command we have now is 'uncommit'. And that
mostly because I had written it in a plugin, where I felt it was okay to
have an interactive command.

> 
>  * it should be possible for the lock holder to discover that they've
> lost the lock; it's not practical to asynchronously signal this to them
> but they could discover it when they try to freshen or release the lock

If possible, but I don't know how much it will actually help them.

I will say that lock holders need to know that they have lost the lock,
rather than unlocking someone else's lock.

I don't know how we do this. Do we just keep an open file descriptor to
the directory, and rename that descriptor? All other methods seem to
have a race condition. (I check that lock/details is correct, but then
when I go to rename, it has actually been modified).

I think the unlocking race condition is potentially very dangerous. I
don't want a bogus client who was sitting on a lock to unlock me, after
I've broken their control.

Would we have to use hard-links for this? Something like:

lock-base/
  pending/
  released/

Create a temp directory in pending, and a temp file, hard-link the temp
file to pending/details, rename pending/tempdir/ to locked/ and rename
pending/temp-file to released/temp-file (or leave it in pending).

Then when you go to rename, you could check the hard-link count on your
temp-file, but you still have the race condition between checking the
count, and actually renaming lock/ to released/temp-dir/

> 
>  * if the lock is not acquired by operations within a timeout, they
> raise an exception.  we could possibly catch that and prompt the user to
> break the lock if they think it's stuck.
> 

We certainly could have the exception contain the lock details, and
present it to the user at that time.

John
=:->

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060220/25796371/attachment.pgp 


More information about the bazaar mailing list