From 74d53120de7c7c48cc54318cb2ac0db0dd135c91 Mon Sep 17 00:00:00 2001
From: Charles Oliver Nutter <headius@headius.com>
Date: Thu, 28 Apr 2016 11:02:41 -0500
Subject: [PATCH] When duping Struct accessor methods, ensure they actually
 dup.

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.
---
 core/src/main/java/org/jruby/RubyStruct.java | 85 ++++++++++++--------
 1 file changed, 51 insertions(+), 34 deletions(-)

diff --git a/core/src/main/java/org/jruby/RubyStruct.java b/core/src/main/java/org/jruby/RubyStruct.java
index 27c8a2e309e..4bbfc5d6c7b 100644
--- a/core/src/main/java/org/jruby/RubyStruct.java
+++ b/core/src/main/java/org/jruby/RubyStruct.java
@@ -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()) {
@@ -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);
+        }
+    }
 }