Patchwork [3,of,3,V2] tags: take lock instead of wlock before writing hgtagsfnodes1 cache

login
register
mail settings
Submitter Pulkit Goyal
Date Sept. 10, 2020, 8:08 a.m.
Message ID <db5aaebba7f2b947baf8.1599725290@workspace>
Download mbox | patch
Permalink /patch/47123/
State Accepted
Headers show

Comments

Pulkit Goyal - Sept. 10, 2020, 8:08 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1599574000 -19800
#      Tue Sep 08 19:36:40 2020 +0530
# Node ID db5aaebba7f2b947baf84c31a82c305202572d66
# Parent  2236c304b8ad360333ee125aa777316628d1e3d5
# EXP-Topic tags-fix
tags: take lock instead of wlock before writing hgtagsfnodes1 cache

This cache is shared across stores and hence we should take store lock before
writing to it. Otherwise there will be race where one share with wlock is
writing to this cache and other share with wlock is trying to read it
simultaneously.

Differential Revision: https://phab.mercurial-scm.org/D9001
Yuya Nishihara - Sept. 11, 2020, 1:07 a.m.
On Thu, 10 Sep 2020 13:38:10 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1599574000 -19800
> #      Tue Sep 08 19:36:40 2020 +0530
> # Node ID db5aaebba7f2b947baf84c31a82c305202572d66
> # Parent  2236c304b8ad360333ee125aa777316628d1e3d5
> # EXP-Topic tags-fix
> tags: take lock instead of wlock before writing hgtagsfnodes1 cache

Queued the series, thanks.

> --- a/mercurial/tags.py
> +++ b/mercurial/tags.py
> @@ -838,7 +838,7 @@ class hgtagsfnodescache(object):
>          repo = self._repo
>  
>          try:
> -            lock = repo.wlock(wait=False)
> +            lock = repo.lock(wait=False)

There wouldn't be no racy lock() -> wlock() here, so this change looks good.

Patch

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -838,7 +838,7 @@  class hgtagsfnodescache(object):
         repo = self._repo
 
         try:
-            lock = repo.wlock(wait=False)
+            lock = repo.lock(wait=False)
         except error.LockError:
             repo.ui.log(
                 b'tagscache',
diff --git a/tests/test-tags.t b/tests/test-tags.t
--- a/tests/test-tags.t
+++ b/tests/test-tags.t
@@ -156,7 +156,7 @@  Tag cache debug info written to blackbox
 Failure to acquire lock results in no write
 
   $ rm -f .hg/cache/tags2-visible .hg/cache/hgtagsfnodes1
-  $ echo 'foo:1' > .hg/wlock
+  $ echo 'foo:1' > .hg/store/lock
   $ hg identify
   b9154636be93 tip
   $ hg blackbox -l 6
@@ -170,7 +170,7 @@  Failure to acquire lock results in no wr
   $ fnodescacheexists
   no fnodes cache
 
-  $ rm .hg/wlock
+  $ rm .hg/store/lock
 
   $ rm -f .hg/cache/tags2-visible .hg/cache/hgtagsfnodes1
   $ hg identify