[MERGE] Add an optional 'token' keyword argument to LockableFiles.lock_write

Andrew Bennetts andrew at canonical.com
Thu Feb 15 06:51:38 GMT 2007


[According to my mail client, your mail client seems to like inserting tabs in
long subject lines.]

Aaron Bentley wrote:
> Andrew Bennetts wrote:
> > Andrew Bennetts wrote:
> >> This adds the low-level support the branch and repository lock sharing that is
> > [...]
> > 
> > Ahem.  And here is the bundle.
> 
> The possible weakness I see here is, how do we know which lock token to
> supply?  It seems like we would need to guess.

You supply the one you have from a previous lock_write.

> Would it be better to supply all known tokens?

I have no idea what you're suggesting here.

The use for this is in the smart server.  The smart server process is stateless
(the only state is what is recorded in the branch/repository) and this is a
property we need to keep.  However, holding a lock is stateful, and so we need
some way for the server to obtain the same state the client is in when it is
doing something for the client.

So if set_revision_history is implemented as a single smart operation, then if a
client does something like:

    branch.lock_write()
    branch.set_revision_history()
    branch.unlock() 

Then this would fail at the moment, because the *server* needs a write lock, but
one is already held (because the client previously did lock_write).  What should
happen instead is that because the server is acting on behalf of the client, the
server should be able to do operations on the branch (and repository) that
require locks that the client holds. 

Here's a simple case that shows how the token will be used:

    branch.lock_write()
      (RemoteBranch calls "Branch.lock_write path/of/branch", and keeps a record of
       the token returned by the server for later requests.)
    branch.unlock()
      (RemoteBranch calls "Branch.unlock path/of/branch lock_token".  On the
       server, it will do branch.lock_write(token=token) to make sure that the
       token matches the actual lock and set the branch object's state
       correctly, then branch.dont_leave_lock_in_place(); branch.unlock() to
       remove the actual lock)

Actually, it's messier than that because locking a branch also locks the
repository, but you get the idea.

Another way to look at it is that these changes allow us to tell Branch and
Repository objects that "you are locked" and "you are no longer locked" without
affecting the locks on disk.  At the moment, the only APIs we have are "take
this lock on disk, and set your internal state to 'locked'" and "set your
internal state to 'unlocked' and release the lock on disk."  If you like, this
change basically makes it possible to synchronise an object's knowledge of lock
state with reality.

I hope I've explained this clearly; it took me a while to understand and
remember what was needed here, but Robert eventually got it to stick :) 

-Andrew.




More information about the bazaar mailing list