[PATCH] Singleton Branches

[John A Meinel]

Aaron Bentley wrote:

> Hi John,
>
> John A Meinel wrote:
>
> >             # On some platforms (*cough* win32 *cough*), upgrading a
> lock requires unlocking it first
>
> On any platform, attempting to upgrade a lock without unlocking first
> can lock you hard.
>
> 1. process A acquires read lock
> 2. process B acquires read lock
> 3. process A attempts to acquire write lock.  Blocked on B.
> 4. process B attempts to acquire write lock.  Blocked on A.  Stalemate.

flock specifically allows you to upgrade a read lock to a write lock. On
win32 you cannot upgrade a lock.

>
> This means that you must unlock before you lock.  But when you unlock,
> it's possible for state to change before you lock again, and it's
> *necessary* for that to be possible, so you don't get into the situation
> described above, because in the situation described above, one of the
> processes has to lose the fight.

Good point, I was just pointing out that the specific function call
doesn't allow it. But certainly we can do the unlock anytime you are
trying to upgrade the lock.
On the other hand, there are quite a few times where upgrading a lock
can be decent.

   1. Process A acquires read lock to do a diff
   2. Process B acquires read lock to do 'revno()'
   3. Process A analyses the diff, realizes something needs to change,
      requests a write lock
   4. Process B finishes and goes away -> Process A acquires the write lock.

Admittedly in your example you can get a deadlock. One semi-interesting
way around it is to have all locks timeout. In a special case
msvcrt.locking() actually tries for 10 seconds, and then fails. You
could do a similar thing, just attempt a write lock in non-blocking mode
for 10 seconds, and if you fail, throw an exception. Pretty easy to do,
and most operations should finish in a reasonable time, or you don't
really want to wait for them anyway.

>
> But you mustn't have the on-disk state change while you think you're
> holding a lock-- that's the point of the lock.  So you can't use a
> singleton to refer to a branch if you want to be able to upgrade the
> lock, because if you already have the branch open and the disk state
> state changes, that would be bad.

Well, one nice possibility of a singleton, is that you can upgrade the
singleton and everyone goes through the same object and sees the same
updates.

>
> If we assume you want to be able to write to the branch sometimes, and
> it's a singleton, then it must always be write-locked.  Otherwise, I
> don't think it can be a singleton, because you have to lose all your
> state when you unlock.  Maybe the flyweight pattern would work for this,
> but not singleton.
>
Well you have to have a singleton inside the application otherwise 2
portions *think* there is no lock held, but in reality there is.

I have no problem making lock() a hidden member, and disallowing
upgrading the lock. But if one portion of the application is holding a
read lock, then the other portion should not request a write lock on the
same branch. I think a singleton is exactly what you want, especially if
you want to disallow upgrading a lock. Otherwise you have no way to know
that you are trying to do an upgrade.

> So this is why I didn't want to retain any state when trying to change
> the locking of a branch.  The intent was to force people to lock a
> writable version of the branch from the get-go, and never upgrade locks
> mid-stream.  I admit I'm not any kind of expert on locking, so if
> there's somthing wrong here, please let me know.
>
> Aaron

How should a long-lived application handle it? I'm thinking they should
re-create the Branch object as they need it. I just now that it is
fairly easy to get 2 Branch() objects created at the same time on the
same directory. baz2bzr requests a new one for each revision, seemingly
without letting go of the old one.

Inside the code, I see a lot of places where it is trying to get a quick
answer, so it does something like "Branch(path).revno()", rather than
have a branch passed in, it just works on the path. Using a singleton,
this becomes a valid request, since you don't request a new lock.

John
=:->

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