RemoteBranch.lock_read() doesn't lock the underlying repository

[Matthew Paul Thomas]

Bazaar 1.6b1, Ubuntu 8.04

> bzr check bzr+ssh://[...]/[...]
Server does not understand Bazaar network protocol 3, reconnecting. (Upgrade the server to avoid this.)
bzr: ERROR: bzrlib.errors.ObjectNotLocked: KnitPackRepository('bzr+ssh://[...]/.bzr/repository/') is not locked

Traceback (most recent call last):
  File "/usr/lib/python2.5/site-packages/bzrlib/commands.py", line 846, in run_bzr_catch_errors
    return run_bzr(argv)
  File "/usr/lib/python2.5/site-packages/bzrlib/commands.py", line 797, in run_bzr
    ret = run(*run_argv)
  File "/usr/lib/python2.5/site-packages/bzrlib/commands.py", line 499, in run_argv_aliases
    return self.run(**all_cmd_args)
  File "/usr/lib/python2.5/site-packages/bzrlib/builtins.py", line 2418, in run
    check(branch_obj, verbose)
  File "/usr/lib/python2.5/site-packages/bzrlib/check.py", line 253, in check
    branch_result = branch.check()
  File "/usr/lib/python2.5/site-packages/bzrlib/decorators.py", line 127, in read_locked
    return unbound(self, *args, **kwargs)
  File "/usr/lib/python2.5/site-packages/bzrlib/branch.py", line 705, in check
    last_revision_id))
  File "/usr/lib/python2.5/site-packages/bzrlib/repository.py", line 1570, in iter_reverse_revision_history
    parents = graph.get_parent_map([next_id])[next_id]
  File "/usr/lib/python2.5/site-packages/bzrlib/graph.py", line 132, in get_parent_map
    new_parents = self._real_provider.get_parent_map(needed)
  File "/usr/lib/python2.5/site-packages/bzrlib/repofmt/pack_repo.py", line 1921, in get_parent_map
    self._pack_collection.ensure_loaded()
  File "/usr/lib/python2.5/site-packages/bzrlib/repofmt/pack_repo.py", line 1340, in ensure_loaded
    raise errors.ObjectNotLocked(self.repo)
ObjectNotLocked: KnitPackRepository('bzr+ssh://[...]/.bzr/repository/') is not locked

bzr 1.6b1 on python 2.5.2 (linux2)
arguments: ['/usr/bin/bzr', 'check', 'bzr+ssh://[...]/[...]']
encoding: 'UTF-8', fsenc: 'UTF-8', lang: 'en_NZ.UTF-8'
plugins:
  launchpad /usr/lib/python2.5/site-packages/bzrlib/plugins/launchpad [unknown]
  loom /home/mpt/.bazaar/plugins/loom [unknown]
  lpreview /home/mpt/.bazaar/plugins/lpreview [unknown]
  pqm /home/mpt/.bazaar/plugins/pqm [1.4.0dev0]
*** Bazaar has encountered an internal error.
    Please report a bug at https://bugs.launchpad.net/bzr/+filebug
    including this traceback, and a description of what you
    were doing when the error occurred.

"bzr cat" produces a similar traceback (originally reported in bug 175570).

[John A Meinel (jameinel)]

Something weird is happening here.

Considering that Branch.check() uses the @needs_read_lock decorator, and you can see the 'read_locked()' function in the traceback. Which means that the branch *should* be read locked, and by extension so should its repository.

Oh wait, isn't there a bug about RemoteBranch.lock_read() not properly locking the underlying repository?

Maybe I didn't actually get it submitted. The problem was that without doing '_ensure_real()' the RemoteBranch would fail to lock its repository. Because it assumed that self._real_branch would be locking it.

Basically a race condition. If you use an api on RemoteBranch that requires it to defer to the self._real_branch, then the repository is properly locked. As we move more functions to be over the RPC, you get more instances where that is not true, and you get failures like this.

[Andrew Bennetts (spiv)]

Adding self.repository.lock_read() to RemoteBranch.lock_read() does fix this problem, but causes a large number of test failures. So it's not completely trivial to fix.

[John A Meinel (jameinel)]

Isn't it a problem that you would end up double-locking the repository? Specifically that you wouldn't necessarily call 'unlock' to match every 'lock_read' call? (because if _real_branch *is* invoked, then it also starts calling lock/unlock based on its algorithm.)

[Andrew Bennetts (spiv)]

I have a fix for this that I'm about to submit for review.

[Andrew Bennetts (spiv)]

Fixed in bzr.dev, and will be in 1.7.