Code review comment for lp:~spiv/bzr/debug-flag-relock

Revision history for this message
Andrew Bennetts (spiv) wrote :

This is an idea I had while looking at bug 389413. It adds a -Drelock debug flag to make noise about cases where we probably should take more care to just hold one lock rather than locking/unlocking/locking-again.

Here's some sample output:

$ ./bzr diff -c 4148 . -Drelock > /dev/null
BzrBranch7('file:///home/andrew/warthogs/bzr/debug-flag-relock/') was read locked again
CHKInventoryRepository('file:///home/andrew/warthogs/bzr/.bzr/repository/') was read locked again
CHKInventoryRepository('file:///home/andrew/warthogs/bzr/.bzr/repository/') was read locked again
CHKInventoryRepository('file:///home/andrew/warthogs/bzr/.bzr/repository/') was read locked again
BzrBranch7('file:///home/andrew/warthogs/bzr/debug-flag-relock/') was read locked again
CHKInventoryRepository('file:///home/andrew/warthogs/bzr/.bzr/repository/') was read locked again
BzrBranch7('file:///home/andrew/warthogs/bzr/debug-flag-relock/') was read locked again
CHKInventoryRepository('file:///home/andrew/warthogs/bzr/.bzr/repository/') was read locked again
CHKInventoryRepository('file:///home/andrew/warthogs/bzr/.bzr/repository/') was read locked again
CHKInventoryRepository('file:///home/andrew/warthogs/bzr/.bzr/repository/') was read locked again
BzrBranch7('file:///home/andrew/warthogs/bzr/debug-flag-relock/') was read locked again

Hopefully that gives some idea of how much we can improve :)

Possibly we should have a more generic hook point for lock/unlock operations so that tests can assert "object X is only locked once", similar to the existing HPSS effort tests... but this seems cheap and useful. (The existing hook points for lockdir lock_acquired/lock_released would only catch lock_write, not lock_read, so they aren't sufficient for this case.)

« Back to merge proposal