[Merge] Two Phase Locking

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

While going through the code, I realized that Branch and WorkingTree
don't agree on the order of locking, and Branch wasn't cleaning up if it
had problems while locking.

So I wrote the attached patch, which tests all branch implementations
and working tree implementations to make sure that they lock in the
correct order, and unlock in the correct order. It instruments the
lock/unlock calls so that they can be recorded/forced to fail.

After discussing with Robert on IRC, we figured that objects should
always lock Other before Self. In a single threaded app, it doesn't
really matter, and there is a nice property of making sure you can lock
yourself before you try to lock other (shallow is checked before deep).
However, if you had a multi-threaded app, it might believe that it was
successfully locked, even though Other wasn't locked yet.
So we went for Other before Self, because it seems more correct.


The only question left is whether calling Branch.unlock() should always
unlock the Repository, even if there is some exception while unlocking
Branch.control_files().

It is cleaner to always unlock Other, so you don't leave the Repository
locked if you (for whatever reason) failed to unlock the WorkingTree.

But going back to the multi-threaded POV, if you expect things are safe
if Self is locked, this could put you in the position of Self being
locked but not Other.

Our code base is rather secure against either locking mechanism. Mostly
because every function that needs a lock asserts '@needs_read_lock' or
'@needs_write_lock'.
So even if you didn't have the Repository locked, the next function
which actually needed it would lock it for you.

The code was always unlocking, so I kept that behavior, and the tests
are asserting that it is occurring. But I want to bring up the
discussion if it would be better to change this.

One other point is that always unlocking means you are trying to do
something while an exception is occurring. So if the sftp link goes
down, you actually try twice, and get 2 exceptions, rather than just one.

Anyway, the attached patch fixes Branch.lock/unlock, and has some tests
for asserting correct behavior, whatever we decide to do. It is present
on 'jam-integration'.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEpZI2JdeBCYSNAAMRAjZXAJ40Tp1D/NwOHVlEFzhBy5MQdViQjgCgz7b6
viCRfdMeQ+52k7RRdNE6FHc=
=XiZn
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: two-phase-locks.patch
Type: text/x-patch
Size: 45674 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060630/91052a69/attachment.bin