Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reset globally cached klass methods in Module#prepend_features #3525

Conversation

ahmadsherif
Copy link
Member

No description provided.

Consider this example:

    class Parent
      def bar
      end
    end

    class Child < Parent
      def bar
        super
      end

      def foo
      end
    end

    Child.new.foo
    Child.new.bar
    # foo and bar are cached in the global cache

    prepended_module = Module.new do
      def foo
      end
    end
    Child.prepend(prepended_module)
    # foo is cleared from the global cache, but bar is not
    # Child hierarchy is now Child < IncludedModule(prepended_module) < IncludedModule(Child) < Parent

    Child.new.bar
    # When bar is looked up here, being still cached globally, Child#bar is the
    # result of the lookup (normally, it should be IncludedModule(Child)#bar).
    # Now when Child#bar is looking for its super, it goes to the first included
    # module, then the second included module, which has the same method table
    # as Child, returning Child#bar, causing non-ending recursive calls.
@jemc
Copy link
Member

jemc commented Nov 18, 2015

@ahmadsherif - Can you take a moment to explain why this is needed when the cache is also cleared from include_modules_from, which is called by prepend_features?

@ahmadsherif
Copy link
Member Author

@jemc Sure. include_modules_from only clears the cache of the newly prepended methods, other cached methods from klass are untouched, which could result in a wrong lookup result (like the example in the commit message).

@yorickpeterse yorickpeterse self-assigned this Nov 18, 2015
@yorickpeterse yorickpeterse merged commit fcf450d into rubinius:master Nov 19, 2015
@yorickpeterse
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants