From 34ae38f4250511a7f8c6a98b1f2acef20edbf31e Mon Sep 17 00:00:00 2001 From: Jeremy Thurgood Date: Sun, 17 Mar 2013 16:49:58 +0200 Subject: [PATCH 1/2] Per-method recursion guard. --- lib-topaz/array.rb | 12 ++++----- tests/objects/test_threadobject.py | 41 ++++++++++++++++++++++++++++++ tests/test_executioncontext.py | 9 ++++--- topaz/executioncontext.py | 25 +++++++++++++----- topaz/objects/fileobject.py | 5 ++-- topaz/objects/threadobject.py | 17 +++++++------ 6 files changed, 81 insertions(+), 28 deletions(-) diff --git a/lib-topaz/array.rb b/lib-topaz/array.rb index b662321b8..9c358bbd4 100644 --- a/lib-topaz/array.rb +++ b/lib-topaz/array.rb @@ -31,7 +31,7 @@ def self.[](*args) def inspect result = "[" - recursion = Thread.current.recursion_guard(self) do + recursion = Thread.current.recursion_guard(:array_inspect, self) do self.each_with_index do |obj, i| if i > 0 result << ", " @@ -143,7 +143,7 @@ def first def flatten(level = -1) list = [] - recursion = Thread.current.recursion_guard(self) do + Thread.current.recursion_guard(:array_flatten, self) do self.each do |item| if level == 0 list << item @@ -155,9 +155,7 @@ def flatten(level = -1) end return list end - if recursion - raise ArgumentError.new("tried to flatten recursive array") - end + raise ArgumentError.new("tried to flatten recursive array") end def flatten!(level = -1) @@ -183,7 +181,7 @@ def ==(other) if self.size != other.size return false end - Thread.current.recursion_guard(self) do + Thread.current.recursion_guard(:array_equals, self) do self.each_with_index do |x, i| if x != other[i] return false @@ -203,7 +201,7 @@ def eql?(other) if self.length != other.length return false end - Thread.current.recursion_guard(self) do + Thread.current.recursion_guard(:array_eqlp, self) do self.each_with_index do |x, i| if !x.eql?(other[i]) return false diff --git a/tests/objects/test_threadobject.py b/tests/objects/test_threadobject.py index 158b61736..64724c196 100644 --- a/tests/objects/test_threadobject.py +++ b/tests/objects/test_threadobject.py @@ -15,3 +15,44 @@ def test_thread_local_storage(self, space): return Thread.current[:a] """) assert space.int_w(w_res) == 1 + + def test_recursion_guard(self, space): + w_res = space.execute(""" + def foo(objs, depth = 0) + obj = objs.shift + recursion = Thread.current.recursion_guard(:foo, obj) do + return foo(objs, depth + 1) + end + if recursion + return [depth, obj] + end + end + return foo([:a, :b, :c, :a, :d]) + """) + w_depth, w_symbol = space.listview(w_res) + assert space.int_w(w_depth) == 3 + assert space.symbol_w(w_symbol) == "a" + + def test_recursion_guard_nested(self, space): + w_res = space.execute(""" + def foo(objs, depth = 0) + obj = objs.shift + Thread.current.recursion_guard(:foo, obj) do + return bar(objs, depth + 1) + end + return [depth, obj] + end + + def bar(objs, depth) + obj = objs.shift + Thread.current.recursion_guard(:bar, obj) do + return foo(objs, depth + 1) + end + return [depth, obj] + end + + return foo([:a, :a, :b, :b, :c, :a, :d, :d]) + """) + w_depth, w_symbol = space.listview(w_res) + assert space.int_w(w_depth) == 5 + assert space.symbol_w(w_symbol) == "a" diff --git a/tests/test_executioncontext.py b/tests/test_executioncontext.py index e8d833262..474405b5d 100644 --- a/tests/test_executioncontext.py +++ b/tests/test_executioncontext.py @@ -1,12 +1,13 @@ class TestExecutionContext(object): def test_recursion_guard(self, space): + f = "my_func" x = object() y = object() - with space.getexecutioncontext().recursion_guard(x) as in_recursion: + with space.getexecutioncontext().recursion_guard(f, x) as in_recursion: assert not in_recursion - with space.getexecutioncontext().recursion_guard(y) as ir2: + with space.getexecutioncontext().recursion_guard(f, y) as ir2: assert not ir2 - with space.getexecutioncontext().recursion_guard(x) as ir3: + with space.getexecutioncontext().recursion_guard(f, x) as ir3: assert ir3 - with space.getexecutioncontext().recursion_guard(x) as ir3: + with space.getexecutioncontext().recursion_guard(f, x) as ir3: assert ir3 diff --git a/topaz/executioncontext.py b/topaz/executioncontext.py index b9799e248..94286e46e 100644 --- a/topaz/executioncontext.py +++ b/topaz/executioncontext.py @@ -13,7 +13,7 @@ def __init__(self): self.regexp_match_cell = None self.w_trace_proc = None self.in_trace_proc = False - self.recursive_objects = {} + self.recursive_calls = {} self.catch_names = {} def settraceproc(self, w_proc): @@ -75,8 +75,13 @@ def gettoprubyframe(self): frame = frame.backref() return frame - def recursion_guard(self, w_obj): - return _RecursionGuardContextManager(self, w_obj) + def recursion_guard(self, func_id, w_obj): + # We need independent recursion detection for different blocks of + # potentially recursive code so that they don't interfere with each + # other and cause false positives. This is only likely to be a problem + # if one recursion-guarded function calls another, but we can't + # guarantee that won't happen. + return _RecursionGuardContextManager(self, func_id, w_obj) def catch_block(self, name): return _CatchContextManager(self, name) @@ -103,21 +108,27 @@ def __exit__(self, exc_type, exc_value, tb): class _RecursionGuardContextManager(object): - def __init__(self, ec, w_obj): + def __init__(self, ec, func_id, w_obj): self.ec = ec + if func_id not in self.ec.recursive_calls: + self.ec.recursive_calls[func_id] = {} + self.recursive_objects = self.ec.recursive_calls[func_id] + self.func_id = func_id self.w_obj = w_obj self.added = False def __enter__(self): - if self.w_obj in self.ec.recursive_objects: + if self.w_obj in self.recursive_objects: return True - self.ec.recursive_objects[self.w_obj] = None + self.recursive_objects[self.w_obj] = None self.added = True return False def __exit__(self, exc_type, exc_value, tb): if self.added: - del self.ec.recursive_objects[self.w_obj] + del self.recursive_objects[self.w_obj] + if not self.recursive_objects: + del self.ec.recursive_calls[self.func_id] class _CatchContextManager(object): diff --git a/topaz/objects/fileobject.py b/topaz/objects/fileobject.py index e8408818d..63b6608c1 100644 --- a/topaz/objects/fileobject.py +++ b/topaz/objects/fileobject.py @@ -144,11 +144,12 @@ def singleton_method_join(self, space, args_w): result = [] for w_arg in args_w: if isinstance(w_arg, W_ArrayObject): - with space.getexecutioncontext().recursion_guard(w_arg) as in_recursion: + ec = space.getexecutioncontext() + with ec.recursion_guard("file_singleton_method_join", w_arg) as in_recursion: if in_recursion: raise space.error(space.w_ArgumentError, "recursive array") string = space.str_w( - W_FileObject.singleton_method_join(self, space, space.listview(w_arg)) + space.send(space.getclassfor(W_FileObject), space.newsymbol("join"), space.listview(w_arg)) ) else: w_string = space.convert_type(w_arg, space.w_string, "to_path", raise_error=False) diff --git a/topaz/objects/threadobject.py b/topaz/objects/threadobject.py index c3e7d7b12..36d64e406 100644 --- a/topaz/objects/threadobject.py +++ b/topaz/objects/threadobject.py @@ -31,13 +31,14 @@ def method_subscript_assign(self, space, key, w_value): return w_value @classdef.method("recursion_guard") - def method_recursion_guard(self, space, w_obj, block): + def method_recursion_guard(self, space, w_identifier, w_obj, block): """ - Detects recursion. If there is none, yield and return false. Else - return true + Calls the block with true if recursion is detected, false otherwise. + It is up to the block to decide what to do in either case. """ - with space.getexecutioncontext().recursion_guard(w_obj) as in_recursion: - if in_recursion: - return space.w_true - space.invoke_block(block, []) - return space.w_false + ec = space.getexecutioncontext() + identifier = space.str_w(w_identifier) + with ec.recursion_guard(identifier, w_obj) as in_recursion: + if not in_recursion: + space.invoke_block(block, []) + return space.newbool(in_recursion) From 942485a199e123514d3c986506b0572c28ddc7fd Mon Sep 17 00:00:00 2001 From: Jeremy Thurgood Date: Sun, 17 Mar 2013 17:41:22 +0200 Subject: [PATCH 2/2] Untag some specs. --- spec/tags/core/array/inspect_tags.txt | 3 --- spec/tags/core/array/reject_tags.txt | 5 ----- spec/tags/core/hash/equal_value_tags.txt | 1 - 3 files changed, 9 deletions(-) diff --git a/spec/tags/core/array/inspect_tags.txt b/spec/tags/core/array/inspect_tags.txt index d493452b2..260367f97 100644 --- a/spec/tags/core/array/inspect_tags.txt +++ b/spec/tags/core/array/inspect_tags.txt @@ -1,6 +1,3 @@ -fails:Array#inspect returns '[]' for an empty Array -fails:Array#inspect calls inspect on its elements and joins the results with commas -fails:Array#inspect represents a recursive element with '[...]' fails:Array#inspect taints the result if the Array is non-empty and tainted fails:Array#inspect taints the result if an element is tainted fails:Array#inspect untrusts the result if the Array is untrusted diff --git a/spec/tags/core/array/reject_tags.txt b/spec/tags/core/array/reject_tags.txt index 518f7d603..1394ba505 100644 --- a/spec/tags/core/array/reject_tags.txt +++ b/spec/tags/core/array/reject_tags.txt @@ -1,7 +1,2 @@ -fails:Array#reject returns a new array without elements for which block is true -fails:Array#reject returns self when called on an Array emptied with #shift -fails:Array#reject properly handles recursive arrays -fails:Array#reject does not return subclass instance on Array subclasses -fails:Array#reject does not retain instance variables fails:Array#reject returns an Enumerator if no block given fails:Array#reject! returns an Enumerator if no block given diff --git a/spec/tags/core/hash/equal_value_tags.txt b/spec/tags/core/hash/equal_value_tags.txt index 517edff17..2b0ec5948 100644 --- a/spec/tags/core/hash/equal_value_tags.txt +++ b/spec/tags/core/hash/equal_value_tags.txt @@ -1,4 +1,3 @@ fails:Hash#== computes equality for complex recursive hashes -fails:Hash#== computes equality for recursive hashes & arrays fails:Hash#== compares the values in self to values in other hash fails:Hash#== compares values with == semantics