[Commented On] D11817: sparse: lock the store when updating requirements config

aalekseyev (Arseniy Alekseyev) phabricator at mercurial-scm.org
Tue Nov 30 19:42:29 UTC 2021


aalekseyev added a comment.


  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:
  
  When cloning:
  
  - 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.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D11817/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D11817

To: aalekseyev, #hg-reviewers, Alphare
Cc: mercurial-patches
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-patches/attachments/20211130/01edd9ea/attachment-0002.html>


More information about the Mercurial-patches mailing list