JRuby

protected method bug: plugin will_paginate shows symptoms

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.1RC3
  • Fix Version/s: JRuby 1.1.2
  • Component/s: Java Integration
  • Labels:
    None
  • Environment:
    rails 2.0.2 jruby 1.1rc3 ubuntu
  • Number of attachments :
    3

Description

This bug was also reported to the author of 'will_paginate'
http://err.lighthouseapp.com/projects/466/tickets/207

This affects the simplest use case:

<%= will_paginate @models %>

Attachments

  1. File
    issue.rb
    06/May/08 8:54 AM
    0.5 kB
    Steen Lehmann
  2. File
    patch_2418.diff
    06/May/08 4:31 PM
    1 kB
    Steen Lehmann
  3. File
    test_frame_self.rb
    09/May/08 5:03 AM
    0.7 kB
    Steen Lehmann

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Stuart Ellidge added a comment -

I can confirm that the same issue exists with JRuby 1.1.1

Show
Stuart Ellidge added a comment - I can confirm that the same issue exists with JRuby 1.1.1
Hide
Permalink
Steen Lehmann added a comment -

Script that works in MRI, fails in JRuby 1.1.1, thus reproducing the issue

Show
Steen Lehmann added a comment - Script that works in MRI, fails in JRuby 1.1.1, thus reproducing the issue
Hide
Permalink
Charles Oliver Nutter added a comment -

Confirmed...really weird.

Show
Charles Oliver Nutter added a comment - Confirmed...really weird.
Hide
Permalink
Steen Lehmann added a comment -

In CallSite.java:102 the call to method.isCallableFrom fails because the current frame has erroneously had its self object set to the string passed into the method (="current"). It would seem highly likely that the code below is the culprit, as it is evaluated prior to isCallableFrom failing:

CallBlock.java:82

public IRubyObject yield(ThreadContext context, IRubyObject value, IRubyObject self,
RubyModule klass, boolean aValue, Binding binding, Block.Type type) {
if (klass == null) { self = binding.getSelf(); // FIXME: We never set this back! binding.getFrame().setSelf(self); }

return callback.call(context, new IRubyObject[] {value}, Block.NULL_BLOCK);
}

Show
Steen Lehmann added a comment - In CallSite.java:102 the call to method.isCallableFrom fails because the current frame has erroneously had its self object set to the string passed into the method (="current"). It would seem highly likely that the code below is the culprit, as it is evaluated prior to isCallableFrom failing: CallBlock.java:82 public IRubyObject yield(ThreadContext context, IRubyObject value, IRubyObject self, RubyModule klass, boolean aValue, Binding binding, Block.Type type) { if (klass == null) { self = binding.getSelf(); // FIXME: We never set this back! binding.getFrame().setSelf(self); } return callback.call(context, new IRubyObject[] {value}, Block.NULL_BLOCK); }
Hide
Permalink
Steen Lehmann added a comment -

Sorry, that's (from CallBlock.java:82 in trunk):

public IRubyObject yield(ThreadContext context, IRubyObject value, IRubyObject self, 
            RubyModule klass, boolean aValue, Binding binding, Block.Type type) {
        if (klass == null) {
            self = binding.getSelf();
            // FIXME: We never set this back!
            binding.getFrame().setSelf(self);
        }
        
        return callback.call(context, new IRubyObject[] {value}, Block.NULL_BLOCK);
    }
Show
Steen Lehmann added a comment - Sorry, that's (from CallBlock.java:82 in trunk): 
public IRubyObject yield(ThreadContext context, IRubyObject value, IRubyObject self, 
            RubyModule klass, boolean aValue, Binding binding, Block.Type type) {
        if (klass == null) {
            self = binding.getSelf();
            // FIXME: We never set this back!
            binding.getFrame().setSelf(self);
        }
        
        return callback.call(context, new IRubyObject[] {value}, Block.NULL_BLOCK);
    }
Hide
Permalink
Steen Lehmann added a comment -

Attaching a patch which seems to fix the issue, while keeping the tests running.

Show
Steen Lehmann added a comment - Attaching a patch which seems to fix the issue, while keeping the tests running.
Hide
Permalink
Steen Lehmann added a comment -

Testcase for inclusion in the unit test suite.

Show
Steen Lehmann added a comment - Testcase for inclusion in the unit test suite.
Hide
Permalink
Charles Oliver Nutter added a comment -

I committed a slightly modified version of this patch that uses try/finally to guarantee the frame self gets set back. Thanks Steen!

Show
Charles Oliver Nutter added a comment - I committed a slightly modified version of this patch that uses try/finally to guarantee the frame self gets set back. Thanks Steen!
Hide
Permalink
Steen Lehmann added a comment -

Thanks Charlie! You could even take out the line "// FIXME: We never set this back!" since now we do

Show
Steen Lehmann added a comment - Thanks Charlie! You could even take out the line "// FIXME: We never set this back!" since now we do

People

Vote (2)
Watch (3)

Dates

  • Created:
    Updated:
    Resolved:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Codehaus. Try JIRA - bug tracking software for your team.