Skip to content

Commit

Permalink
When duping Struct accessor methods, ensure they actually dup.
Browse files Browse the repository at this point in the history
Partial fix for #3834

The main issue here is that we frequently return this for
DynamicMethod#dup, rather than return a new instance, and then
this gets modified by code expecting it to be something different.
This breaks reflective code because many reflective methods will
expect the methods in a module's method table to say their
implementationClass is that module.

The case in #3834 broke because the accessor methods in Struct
have this "fake" dup logic, so when the struct class itself was
duplicated, the methods got modified implementationClass = clone
and seemed to "disappear" from the original.

This is only a partial fix because it just fixes those accessors
in Struct. The proper fix would be to make sure DynamicMethod#dup
returns a real new object for all DynamicMethod (except "dead"
ones like UndefinedMethod). This may also fix (or make easier to
fix) issues like #2988, where a dup'ed module's methods do not
reflect their new implementationClass.

Specs pending.
  • Loading branch information
headius committed Apr 28, 2016
1 parent 85212ad commit 74d5312
Showing 1 changed file with 51 additions and 34 deletions.
85 changes: 51 additions & 34 deletions core/src/main/java/org/jruby/RubyStruct.java
Expand Up @@ -229,40 +229,8 @@ public static RubyClass newInstance(IRubyObject recv, IRubyObject[] args, Block
final String memberName = args[i].asJavaString();
// if we are storing a name as well, index is one too high for values
final int index = (name == null && !nilName) ? i : i - 1;
newStruct.addMethod(memberName, new DynamicMethod(newStruct, Visibility.PUBLIC) {
@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
Arity.checkArgumentCount(context.runtime, name, args, 0, 0);
return ((RubyStruct)self).get(index);
}

@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name) {
return ((RubyStruct)self).get(index);
}

@Override
public DynamicMethod dup() {
return this;
}
});
newStruct.addMethod(memberName + '=', new DynamicMethod(newStruct, Visibility.PUBLIC) {
@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
Arity.checkArgumentCount(context.runtime, name, args, 1, 1);
return ((RubyStruct)self).set(args[0], index);
}

@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg) {
return ((RubyStruct)self).set(arg, index);
}

@Override
public DynamicMethod dup() {
return this;
}
});
newStruct.addMethod(memberName, new Accessor(newStruct, index));
newStruct.addMethod(memberName + '=', new Mutator(newStruct, index));
}

if (block.isGiven()) {
Expand Down Expand Up @@ -849,4 +817,53 @@ public IRubyObject initialize_copy(IRubyObject arg) {
return this;
}

private static class Accessor extends DynamicMethod {
private final int index;

public Accessor(RubyClass newStruct, int index) {
super(newStruct, Visibility.PUBLIC);
this.index = index;
}

@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
Arity.checkArgumentCount(context.runtime, name, args, 0, 0);
return ((RubyStruct)self).get(index);
}

@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name) {
return ((RubyStruct)self).get(index);
}

@Override
public DynamicMethod dup() {
return new Accessor((RubyClass) getImplementationClass(), index);
}
}

private static class Mutator extends DynamicMethod {
private final int index;

public Mutator(RubyClass newStruct, int index) {
super(newStruct, Visibility.PUBLIC);
this.index = index;
}

@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject[] args, Block block) {
Arity.checkArgumentCount(context.runtime, name, args, 1, 1);
return ((RubyStruct)self).set(args[0], index);
}

@Override
public IRubyObject call(ThreadContext context, IRubyObject self, RubyModule clazz, String name, IRubyObject arg) {
return ((RubyStruct)self).set(arg, index);
}

@Override
public DynamicMethod dup() {
return new Accessor((RubyClass) getImplementationClass(), index);
}
}
}

0 comments on commit 74d5312

Please sign in to comment.