Patchwork [stable] fileset, revset: do not use global parser object for thread safety

login
register
mail settings
Submitter Yuya Nishihara
Date Dec. 21, 2013, 4:15 a.m.
Message ID <20131221131500.f3021733a8450575ec8a44ff@tcha.org>
Download mbox | patch
Permalink /patch/3225/
State Accepted
Commit 61a47fd64f308ecf18696f06f84918bef9564d7c
Headers show

Comments

Yuya Nishihara - Dec. 21, 2013, 4:15 a.m.
Hi,

This problem was originally reported as a bug of TortoiseHg, but hgweb would
have the same issue.

https://bitbucket.org/tortoisehg/thg/issue/3533/

script to reproduce the exception:

import threading
from mercurial import revset

spec = '()'
for i in xrange(10):
    spec = '(%s or %d)' % (spec, i)
print spec

for _i in xrange(50):
    th = threading.Thread(target=lambda: revset.parse(spec))
    th.start()

# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1387597459 -32400
#      Sat Dec 21 12:44:19 2013 +0900
# Branch stable
# Node ID 61a47fd64f308ecf18696f06f84918bef9564d7c
# Parent  d4be314b20711b6ecfa7a6f8b46970231134004a
fileset, revset: do not use global parser object for thread safety

parse() cannot be called at the same time because a parser object keeps its
states.  This is no problem for command-line hg client, but it would cause
strange errors in multi-threaded hgweb.

Creating parser object is not too expensive.

original:
% python -m timeit -s 'from mercurial import revset' 'revset.parse("0::tip")'
100000 loops, best of 3: 11.3 usec per loop

thread-safe:
% python -m timeit -s 'from mercurial import revset' 'revset.parse("0::tip")'
100000 loops, best of 3: 13.1 usec per loop
Augie Fackler - Jan. 1, 2014, 11:28 p.m.
On Dec 20, 2013, at 11:15 PM, Yuya Nishihara <yuya@tcha.org> wrote:

> Hi,
> 
> This problem was originally reported as a bug of TortoiseHg, but hgweb would
> have the same issue.
> 
> https://bitbucket.org/tortoisehg/thg/issue/3533/
> 
> script to reproduce the exception:
> 
> import threading
> from mercurial import revset
> 
> spec = '()'
> for i in xrange(10):
>    spec = '(%s or %d)' % (spec, i)
> print spec
> 
> for _i in xrange(50):
>    th = threading.Thread(target=lambda: revset.parse(spec))
>    th.start()
> 
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1387597459 -32400
> #      Sat Dec 21 12:44:19 2013 +0900
> # Branch stable
> # Node ID 61a47fd64f308ecf18696f06f84918bef9564d7c
> # Parent  d4be314b20711b6ecfa7a6f8b46970231134004a
> fileset, revset: do not use global parser object for thread safety

LGTM, queueing for stable.

> 
> parse() cannot be called at the same time because a parser object keeps its
> states.  This is no problem for command-line hg client, but it would cause
> strange errors in multi-threaded hgweb.
> 
> Creating parser object is not too expensive.
> 
> original:
> % python -m timeit -s 'from mercurial import revset' 'revset.parse("0::tip")'
> 100000 loops, best of 3: 11.3 usec per loop
> 
> thread-safe:
> % python -m timeit -s 'from mercurial import revset' 'revset.parse("0::tip")'
> 100000 loops, best of 3: 13.1 usec per loop
> 
> diff --git a/mercurial/fileset.py b/mercurial/fileset.py
> --- a/mercurial/fileset.py
> +++ b/mercurial/fileset.py
> @@ -78,7 +78,9 @@ def tokenize(program):
>         pos += 1
>     yield ('end', None, pos)
> 
> -parse = parser.parser(tokenize, elements).parse
> +def parse(expr):
> +    p = parser.parser(tokenize, elements)
> +    return p.parse(expr)
> 
> def getstring(x, err):
>     if x and (x[0] == 'string' or x[0] == 'symbol'):
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1880,7 +1880,9 @@ def findaliases(ui, tree):
>         aliases[alias.name] = alias
>     return _expandaliases(aliases, tree, [], {})
> 
> -parse = parser.parser(tokenize, elements).parse
> +def parse(spec):
> +    p = parser.parser(tokenize, elements)
> +    return p.parse(spec)
> 
> def match(ui, spec):
>     if not spec:
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/fileset.py b/mercurial/fileset.py
--- a/mercurial/fileset.py
+++ b/mercurial/fileset.py
@@ -78,7 +78,9 @@  def tokenize(program):
         pos += 1
     yield ('end', None, pos)
 
-parse = parser.parser(tokenize, elements).parse
+def parse(expr):
+    p = parser.parser(tokenize, elements)
+    return p.parse(expr)
 
 def getstring(x, err):
     if x and (x[0] == 'string' or x[0] == 'symbol'):
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1880,7 +1880,9 @@  def findaliases(ui, tree):
         aliases[alias.name] = alias
     return _expandaliases(aliases, tree, [], {})
 
-parse = parser.parser(tokenize, elements).parse
+def parse(spec):
+    p = parser.parser(tokenize, elements)
+    return p.parse(spec)
 
 def match(ui, spec):
     if not spec: