[GR-18163] Avoid recursion when redefining Warning#warn (#2109)

PullRequest: truffleruby/2035

Author: eregon

CHANGELOG.md

@@ -45,6 +45,7 @@ Compatibility:
 * Support refinements for `#to_s` called by string interpolation (#2110, @ssnickolay)
 * Module#using raises error in method scope (#2112, @ssnickolay)
 * `File#path` now returns a new mutable String on every call like MRI (#2115).
+* Avoid infinite recursion when redefining `Warning#warn` and calling `Kernel#warn` (#2109).
 
 Performance:
 

spec/ruby/core/kernel/warn_spec.rb

@@ -197,4 +197,35 @@ def o.to_sym; :deprecated; end
     -> { warn(**h) }.should_not complain(verbose: true)
     -> { warn('foo', **h) }.should complain("foo\n")
   end
+
+  it "does not call Warning.warn if self is the Warning module" do
+    # RubyGems redefines Kernel#warn so we need to use a subprocess and disable RubyGems here
+    code = <<-RUBY
+    def Warning.warn(*args, **kwargs)
+      raise 'should not be called'
+    end
+    Kernel.instance_method(:warn).bind(Warning).call('Kernel#warn spec edge case')
+    RUBY
+    out = ruby_exe(code, args: "2>&1", options: "--disable-gems")
+    out.should == "Kernel#warn spec edge case\n"
+    $?.should.success?
+  end
+
+  it "avoids recursion if Warning#warn is redefined and calls super" do
+    # This works because of the spec above, which is the workaround for it.
+    # Note that redefining Warning#warn is a mistake which would naturally end in infinite recursion,
+    # Warning.extend Module.new { def warn } should be used instead.
+    # RubyGems redefines Kernel#warn so we need to use a subprocess and disable RubyGems here
+    code = <<-RUBY
+    module Warning
+      def warn(*args)
+        super
+      end
+    end
+    warn "avoid infinite recursion"
+    RUBY
+    out = ruby_exe(code, args: "2>&1", options: "--disable-gems")
+    out.should == "avoid infinite recursion\n"
+    $?.should.success?
+  end
 end

spec/tags/core/kernel/warn_tags.txt

@@ -1,2 +1,4 @@
 slow:Kernel#warn :uplevel keyword argument shows the caller of #require and not #require itself without RubyGems
 slow:Kernel#warn :uplevel keyword argument shows the caller of #require and not #require itself with RubyGems loaded
+slow:Kernel#warn does not call Warning.warn if self is the Warning module
+slow:Kernel#warn avoids recursion if Warning#warn is redefined and calls super

src/main/ruby/truffleruby/core/kernel.rb

@@ -643,7 +643,7 @@ def untrace_var(name, cmd=undefined)
   def warn(*messages, uplevel: undefined)
     if !Primitive.nil?($VERBOSE) && !messages.empty?
       prefix = if Primitive.undefined?(uplevel)
-                 +''
+                 ''
                else
                  uplevel = Primitive.rb_to_int(uplevel)
                  raise ArgumentError, "negative level (#{uplevel})" unless uplevel >= 0
@@ -662,13 +662,22 @@ def warn(*messages, uplevel: undefined)
                  if caller
                    "#{caller.path}:#{caller.lineno}: warning: "
                  else
-                   +'warning: '
+                   'warning: '
                  end
                end
 
-      stringio = Truffle::StringOperations::SimpleStringIO.new(prefix)
+      stringio = Truffle::StringOperations::SimpleStringIO.new(+prefix)
       Truffle::IOOperations.puts(stringio, *messages)
-      Warning.warn(stringio.string)
+      message = stringio.string
+
+      if Primitive.object_equal(self, Warning) # avoid recursion when redefining Warning#warn
+        unless message.encoding.ascii_compatible?
+          raise Encoding::CompatibilityError, "ASCII incompatible encoding: #{message.encoding}"
+        end
+        $stderr.write message
+      else
+        Warning.warn(message)
+      end
     end
     nil
   end