More comprehensive recursion guard.

lib-topaz/array.rb

@@ -27,7 +27,7 @@ def initialize(size_or_arr = nil, obj = nil, &block)
 
   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 << ", "
@@ -138,7 +138,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
@@ -150,9 +150,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)
@@ -178,7 +176,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
@@ -198,7 +196,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

lib-topaz/bootstrap.rb

@@ -5,6 +5,7 @@
   load(File.join(lib_topaz, file))
 end
 
+load_bootstrap.call("thread.rb")
 load_bootstrap.call("array.rb")
 load_bootstrap.call("class.rb")
 load_bootstrap.call("comparable.rb")

lib-topaz/thread.rb

@@ -0,0 +1,26 @@
+class Thread
+  def recursion_guard_outer(identifier, obj, &block)
+    # We want to throw something less likely to be caught accidentally outside
+    # our own code than the recursion identifier. Ideally this should be an
+    # object that is unique to this particular recursion guard. Since doing
+    # that properly requires pushing extra state all the way up into
+    # ExecutionContext, we do this instead.
+    throw_symbol = "__recursion_guard_#{identifier}".to_sym
+
+    if self.in_recursion_guard?(identifier)
+      self.recursion_guard(identifier, obj) do
+        yield
+        return false
+      end
+      throw(throw_symbol)
+    else
+      self.recursion_guard(identifier, obj) do
+        catch(throw_symbol) do
+          yield
+          return false
+        end
+        return true
+      end
+    end
+  end
+end

tests/objects/test_threadobject.py

@@ -15,3 +15,59 @@ 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"
+
+    def test_recursion_guard_outer(self, space):
+        w_res = space.execute("""
+        def foo(objs, depth = 0)
+          obj = objs.shift
+          Thread.current.recursion_guard_outer(:foo, obj) do
+            return foo(objs, depth + 1)
+          end
+          return [depth, obj]
+        end
+        return foo([:a, :b, :c, :a, :d])
+        """)
+        w_depth, w_symbol = space.listview(w_res)
+        assert space.int_w(w_depth) == 0
+        assert space.symbol_w(w_symbol) == "a"

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

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):

topaz/objects/fileobject.py

@@ -141,11 +141,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)

topaz/objects/threadobject.py

@@ -31,13 +31,22 @@ 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, [])
+        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)
+
+    @classdef.method("in_recursion_guard?")
+    def method_in_recursion_guardp(self, space, w_identifier):
+        ec = space.getexecutioncontext()
+        identifier = space.str_w(w_identifier)
+        if identifier in ec.recursive_calls:
+            return space.w_true
         return space.w_false