Patchwork [1,of,7,STABLE] subrepo: ensure "lock.release()" execution at the end of "storeclean()"

# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1403191279 -32400
#      Fri Jun 20 00:21:19 2014 +0900
# Branch stable
# Node ID 15bc43d3360a4826cab2c3b93e6bf2cfafbcf070
# Parent  b2dc026a9bd2620a0b8c1f28caf5a608352960ab
subrepo: ensure "lock.release()" execution at the end of "storeclean()"

Before this patch, "lock.release()" for "self._repo" in "storeclean()"
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
"storeclean()" by moving it into "finally" clause.

This patch chooses moving almost all lines in "storeclean()" into
"_storeclean()" instead of indenting them for "try/finally" clauses,
to keep diff simple for review-ability.

At Fri, 20 Jun 2014 00:43:16 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1403191279 -32400
> #      Fri Jun 20 00:21:19 2014 +0900
> # Branch stable
> # Node ID 15bc43d3360a4826cab2c3b93e6bf2cfafbcf070
> # Parent  b2dc026a9bd2620a0b8c1f28caf5a608352960ab
> subrepo: ensure "lock.release()" execution at the end of "storeclean()"

This patch series is posted as STABLE for preventive bug fixing,
because it may be difficult to avoid them if they once occur.

But I also agree queuing them into DEFAULT, because of rare
possibility of these problems.


> Before this patch, "lock.release()" for "self._repo" in "storeclean()"
> 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
> "storeclean()" by moving it into "finally" clause.
> 
> This patch chooses moving almost all lines in "storeclean()" into
> "_storeclean()" instead of indenting them for "try/finally" clauses,
> to keep diff simple for review-ability.
> 
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -521,8 +521,14 @@ class hgsubrepo(abstractsubrepo):
>          self._initrepo(r, state[0], create)
>  
>      def storeclean(self, path):
> +        lock = self._repo.lock()
> +        try:
> +            return self._storeclean(path)
> +        finally:
> +            lock.release()
> +
> +    def _storeclean(self, path):
>          clean = True
> -        lock = self._repo.lock()
>          itercache = self._calcstorehash(path)
>          try:
>              for filehash in self._readstorehashcache(path):
> @@ -539,7 +545,6 @@ class hgsubrepo(abstractsubrepo):
>                  clean = False
>              except StopIteration:
>                  pass
> -        lock.release()
>          return clean
>  
>      def _calcstorehash(self, remotepath):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 


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