[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