Wow, looks I just didn't run all tests here. Sorry!
Investigating this, I went pretty deep into a rabbit hole and may need rescuing.
The bug seems to go like this:
- hg.py:868: lock is obtained and stored into destlock variable
- a few lines later, the repo is actually created and the python object representing it is made by destpeer = peer(srcrepo, peeropts, dest) (which makes it unaware of the lock being held)
- hg.py:1038: _update is called on a repo that still doesn't know that it's locked; since _update is where sparse hooks in, updateconfig runs and deadlocks
Note that a few lines later (hg.py:1041-1047) the repo actually "learns" that it's locked (in a racy way).
Intuitively it seems good to "tell" the repo that it's locked from the very start, when calling peer or immediately after. But since this is the first time I see all of this code I can't make that call.
Another confusing thing is that during the clone we don't take wlock at all (other than in sparse), even though it seems reasonable to expect everything to be locked during the clone.
Taking wlock after the lock is supposed to be wrong (deadlock-prone), and yet here we are effectively doing it (clone takes the lock from the very start, and then sparse does it during clone).
Any advice would be appreciated.
- I agree that we should take the wlock in addition to the lock as we will be running an upgrade. (this should be a different commit, prior to the other one)
- the best way to get out of your trouble is to make the "new" peer (created after status) aware of the locking. Something like the logic below seems "suitable", Since this only happens in this specific cases, it seems like a bad idea to alter too much of the main peer API (we could still have a dedicated method on the repository for that). Same would have to be done for the wlock too
(I did not run this code, and it is not intended to be the final state.)
diff --git a/mercurial/hg.py b/mercurial/hg.py --- a/mercurial/hg.py +++ b/mercurial/hg.py @@ -873,6 +873,13 @@ def clone( # we need to re-init the repo after manually copying the data # into it destpeer = peer(srcrepo, peeropts, dest) + + # make the peer aware that is it already locked + # + # important: + # + # We still need to release that lock at the end of the function + destpeer.local()._lockref = weakref.ref(destlock) srcrepo.hook( b'outgoing', source=b'clone', node=srcrepo.nodeconstants.nullhex )