Patchwork [09,of,10] subrepo: ensure "lock.release()" execution at the end of "_cachestorehash()"

login
register
mail settings
Submitter Katsunori FUJIWARA
Date May 28, 2014, 3 p.m.
Message ID <c55208e571d22bc350a9.1401289221@juju>
Download mbox | patch
Permalink /patch/4892/
State Superseded
Commit b9e8fdc35daf103fa56d2fc6ff7c70e6d07abfdd
Headers show

Comments

Katsunori FUJIWARA - May 28, 2014, 3 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1401288802 -32400
#      Wed May 28 23:53:22 2014 +0900
# Node ID c55208e571d22bc350a9c12193403f7f2e730db5
# Parent  1870030d4dd84398ad88caa9579cf9a4480ea980
subrepo: ensure "lock.release()" execution at the end of "_cachestorehash()"

Before this patch, "lock.release()" for "self._repo" in
"_cachestorehash()" of "hgsubrepo" may not be executed, if unexpected
exception is raised, because it isn't executed in "finally" clause.

This patch ensures "lock.release()" execution at the end of
"_cachestorehash()" by moving it into "finally" clause.
Katsunori FUJIWARA - May 28, 2014, 3:22 p.m.
At Thu, 29 May 2014 00:00:21 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1401288802 -32400
> #      Wed May 28 23:53:22 2014 +0900
> # Node ID c55208e571d22bc350a9c12193403f7f2e730db5
> # Parent  1870030d4dd84398ad88caa9579cf9a4480ea980
> subrepo: ensure "lock.release()" execution at the end of "_cachestorehash()"
> 
> Before this patch, "lock.release()" for "self._repo" in
> "_cachestorehash()" of "hgsubrepo" may not be executed, if unexpected
> exception is raised, because it isn't executed in "finally" clause.
> 
> This patch ensures "lock.release()" execution at the end of
> "_cachestorehash()" by moving it into "finally" clause.
> 
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -568,10 +568,12 @@ class hgsubrepo(abstractsubrepo):
>          '''
>          cachefile = _getstorehashcachename(remotepath)
>          lock = self._repo.lock()
> -        storehash = list(self._calcstorehash(remotepath))
> -        vfs = self._cachestorehashvfs
> -        vfs.writelines(cachefile, storehash, mode='w', notindexed=True)
> -        lock.release()
> +        try:
> +            storehash = list(self._calcstorehash(remotepath))
> +            vfs = self._cachestorehashvfs
> +            vfs.writelines(cachefile, storehash, mode='w', notindexed=True)
> +        finally:
> +            lock.release()
>  
>      @annotatesubrepoerror
>      def _initrepo(self, parentrepo, source, create):

I included this (#9) and next (#10) patches fixing "lock.release()"
problem in this series for NON-STABLE branch, because:

  - exception raising while reading/writing in these case seem to be
    very rare

  - in ordinary use-cases, lock should be released via destructor of
    the "lock" class at the end of each "hg" command invocations and
    not so serious (shouldn't it ?)

  - to fix these problems on stable exactly, these should also imply
    fixing "closing fd" problems fixed by #5 and #8 in this series,
    and make changes/steps complicate unexpectedly


I'll re-send them for STABLE branch separately, if I should do so.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -568,10 +568,12 @@  class hgsubrepo(abstractsubrepo):
         '''
         cachefile = _getstorehashcachename(remotepath)
         lock = self._repo.lock()
-        storehash = list(self._calcstorehash(remotepath))
-        vfs = self._cachestorehashvfs
-        vfs.writelines(cachefile, storehash, mode='w', notindexed=True)
-        lock.release()
+        try:
+            storehash = list(self._calcstorehash(remotepath))
+            vfs = self._cachestorehashvfs
+            vfs.writelines(cachefile, storehash, mode='w', notindexed=True)
+        finally:
+            lock.release()
 
     @annotatesubrepoerror
     def _initrepo(self, parentrepo, source, create):