oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3.03k stars 185 forks source link

`super` calls the wrong method when containing module mixed in twice in the same object hierarchy #2757

Open nirvdrum opened 2 years ago

nirvdrum commented 2 years ago

I've been trying to track down an infinite loop that occurs in a Rails application with TruffleRuby, but not MRI. I finally tracked it down to super not calling the correct method. The case I narrowed down is when a mixin defines a method that calls super and that mixin appears twice in the same class hierarchy. I haven't debugged TruffleRuby to determine if there's a more fundamental rule at play, but I wanted to file the issue with my findings thus far.

module Mixin
  def process_action
    puts "Mixin -- #{caller_locations(1, 1)}"
    super
  end
end

module ActionController
  class Metal
  end

  class Base < Metal
  end
end

class ActionController::Metal
end

class ActionController::Base < ActionController::Metal
end

class ApplicationController < ActionController::Base
end

ActionController::Base.include(Mixin)
ActionController::Metal.include(Mixin)

ApplicationController.new.send(:process_action)

Running with MRI 3.1.2, I get:

> ruby mixin.rb
Mixin -- ["mixin.rb:28:in `<main>'"]
Mixin -- ["mixin.rb:4:in `process_action'"]
mixin.rb:4:in `process_action': super: no superclass method `process_action' for #<ApplicationController:0x000000010464d3b8> (NoMethodError)
    from mixin.rb:4:in `process_action'
    from mixin.rb:28:in `<main>'

Running with TruffleRuby b23ca0d4f728103748fa53874c1c73b5bee20274 (23.0.0-dev), I get:

Mixin -- ["mixin.rb:28:in `<main>'"]
Mixin -- ["mixin.rb:4:in `process_action'"]
Mixin -- ["mixin.rb:4:in `process_action'"]
Mixin -- ["mixin.rb:4:in `process_action'"]
Mixin -- ["mixin.rb:4:in `process_action'"]
Mixin -- ["mixin.rb:4:in `process_action'"]
...
[ruby] WARNING StackOverflowError
mixin.rb:2:in `process_action': stack level too deep (SystemStackError)
    from org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.doInvoke(OptimizedCallTarget.java:549)
nirvdrum commented 2 years ago

I got as far as determining it's not simply an issue with the cache guards in LookupSuperMethodNode. ModuleOperations.lookupSuperMethod is returning the wrong result.

eregon commented 2 years ago

Thank you for the report. Right, including the mixin in Base before Metal (with Base < Metal) is what allows the mixin to be twice in the ancestors, and that breaks our super lookup algorithm. (Reversing the include's would only include it once and cause no problem, but obviously not a general solution) So far we assumed a given method is only once in a super chain, but this is not true in this situation. Specifically what's hard here is when we are in Mixin#process_action, we cannot know from the current frame where we should go next, there are two possibilities and the frame doesn't currently contain any info to tell us which one is correct.

It's the same issue I mentioned here: https://github.com/oracle/truffleruby/issues/2558#issuecomment-1028102508

I'll take a look at what CRuby does. I suspect they store the iclass (ModuleChain for us) on the stack/in the frame, either always or from the first super call. That gives enough information, but it's extra work for all calls/all super calls (but then extra checks for the first super) and it takes extra memory. I think there is no choice though.