[RFC] Add run_cleanup, do_with_cleanup

Andrew Bennetts andrew.bennetts at canonical.com
Fri Sep 25 02:19:22 BST 2009


Martin Pool wrote:
[...]
> OK, so +1 from me, but the proof will be in how it would handle the
> bugs reported as TooMany*.  It might be worth grepping for them and
> looking at their call stacks.

So, I've spent some time looking at the TooMany* bug reports.  Here's a summary
of causes:

 * WorkingTree.pull has error -> then runs unlock in finally
 * Branch.push has error -> then run unlock in finally
 * Repo.fetch has error during stream -> then run unlock in finally
   (e.g. due to buggy stream from old server sending cross-format to 2a)
 * commit has an error -> then runs a bunch of unlocks in a finally

So those are all a case of unlock doomed to failure by prior error, then
obscures that prior error.

There is one other case, though:

 * WorkingTree.commit invokes -> BzrBranch5.get_master_branch -> Branch.open ->
   BzrDir.open -> do_catching_redirections... eventually 'BzrDir.open' RPC fails
   with TooMany* error.

This one doesn't appear to be a cleanup error, but some sort of failure about
charging onwards after an error, when probably we shouldn't?  I'm not sure
exactly what's going on there, but I suspect a bug in the way we find formats in
BzrDir.open.  Anyway, this one is not a bug during cleanup AFAICT.

So, it appears we would get excellent mileage out of changing unlock on the core
objects (Branch, WorkingTree, etc) to suppress at least most errors, although
probably we still want to allow LockNotHeld and maybe LockBroken to raise, not
just warn?




More information about the bazaar mailing list