truffleruby icon indicating copy to clipboard operation
truffleruby copied to clipboard

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

Open nirvdrum opened this issue 3 years ago • 2 comments

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 avatar Oct 07 '22 05:10 nirvdrum

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.

nirvdrum avatar Oct 07 '22 05:10 nirvdrum

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.

eregon avatar Oct 07 '22 10:10 eregon