Merge pull request #7739 from headius/no_marshal_finalizer

headius · web-flow · commit 0b5918c54d1e · 2023-03-29T16:33:33.000-05:00
Don't try to marshal non-serializable variables
diff --git a/core/src/main/java/org/jruby/RubyBasicObject.java b/core/src/main/java/org/jruby/RubyBasicObject.java
@@ -1331,6 +1331,20 @@ public List<Variable<Object>> getVariableList() {
         return list;
     }
 
+    /**
+     * @see IRubyObject#getMarshalVariableList()
+     */
+    public List<Variable<Object>> getMarshalVariableList() {
+        Map<String, VariableAccessor> ivarAccessors = metaClass.getVariableAccessorsForRead();
+        ArrayList<Variable<Object>> list = new ArrayList<>(ivarAccessors.size());
+        for (Map.Entry<String, VariableAccessor> entry : ivarAccessors.entrySet()) {
+            Object value = entry.getValue().get(this);
+            if (value == null || !(value instanceof Serializable)) continue;
+            list.add(new VariableEntry<>(entry.getKey(), value));
+        }
+        return list;
+    }
+
     /**
      * Gets a name list of all variables in this object.
      */
diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -1269,7 +1269,7 @@ public void marshalTo(Ruby runtime, Object obj, RubyClass type, MarshalStream ma
             IRubyObject object = (IRubyObject) obj;
 
             marshalStream.registerLinkTarget(object);
-            marshalStream.dumpVariables(object.getVariableList());
+            marshalStream.dumpVariables(object.getMarshalVariableList());
         }
 
         @Override
diff --git a/core/src/main/java/org/jruby/RubyException.java b/core/src/main/java/org/jruby/RubyException.java
@@ -170,7 +170,7 @@ public static RubyClass createExceptionClass(Ruby runtime) {
         public void marshalTo(Ruby runtime, RubyException exc, RubyClass type,
                               MarshalStream marshalStream) throws IOException {
             marshalStream.registerLinkTarget(exc);
-            List<Variable<Object>> attrs = exc.getVariableList();
+            List<Variable<Object>> attrs = exc.getMarshalVariableList();
             attrs.add(new VariableEntry<>("mesg", exc.getMessage()));
             attrs.add(new VariableEntry<>("bt", exc.getBacktrace()));
             marshalStream.dumpVariables(attrs);
diff --git a/core/src/main/java/org/jruby/RubyProcess.java b/core/src/main/java/org/jruby/RubyProcess.java
@@ -172,7 +172,7 @@ public void marshalTo(Ruby runtime, Object obj, RubyClass type,
             RubyStatus status = (RubyStatus) obj;
 
             marshalStream.registerLinkTarget(status);
-            List<Variable<Object>> attrs = status.getVariableList();
+            List<Variable<Object>> attrs = status.getMarshalVariableList();
 
             attrs.add(new VariableEntry("status", runtime.newFixnum(status.status)));
             attrs.add(new VariableEntry("pid", runtime.newFixnum(status.pid)));
diff --git a/core/src/main/java/org/jruby/RubyRange.java b/core/src/main/java/org/jruby/RubyRange.java
@@ -1143,7 +1143,7 @@ public void marshalTo(Ruby runtime, Object obj, RubyClass type,
             RubyRange range = (RubyRange) obj;
 
             marshalStream.registerLinkTarget(range);
-            List<Variable<Object>> attrs = range.getVariableList();
+            List<Variable<Object>> attrs = range.getMarshalVariableList();
 
             attrs.add(new VariableEntry<Object>("excl", range.isExclusive ? runtime.getTrue() : runtime.getFalse()));
             attrs.add(new VariableEntry<Object>("begin", range.begin));
diff --git a/core/src/main/java/org/jruby/RubySystemCallError.java b/core/src/main/java/org/jruby/RubySystemCallError.java
@@ -139,7 +139,7 @@ public void marshalTo(Ruby runtime, Object obj, RubyClass type,
             RubySystemCallError exc = (RubySystemCallError) obj;
             marshalStream.registerLinkTarget(exc);
             
-            List<Variable<Object>> attrs = exc.getVariableList();
+            List<Variable<Object>> attrs = exc.getMarshalVariableList();
             attrs.add(new VariableEntry<Object>(
                     "mesg", exc.message == null ? runtime.getNil() : exc.message));
             attrs.add(new VariableEntry<Object>("errno", exc.errno));
diff --git a/core/src/main/java/org/jruby/runtime/builtin/IRubyObject.java b/core/src/main/java/org/jruby/runtime/builtin/IRubyObject.java
@@ -361,6 +361,13 @@ static IRubyObject[] array(int length) {
      */
     List<Variable<Object>> getVariableList();
 
+    /**
+     * @return a list of all marshalable variables (ivar/cvar/constant/internal)
+     */
+    default List<Variable<Object>> getMarshalVariableList() {
+        return getVariableList();
+    }
+
     //
     // INSTANCE VARIABLE METHODS
     //
diff --git a/core/src/main/java/org/jruby/runtime/marshal/MarshalStream.java b/core/src/main/java/org/jruby/runtime/marshal/MarshalStream.java
@@ -161,7 +161,7 @@ private List<Variable<Object>> getVariables(IRubyObject value) throws IOExceptio
                     // object has instance vars and isn't a class, get a snapshot to be marshalled
                     // and output the ivar header here
 
-                    variables = value.getVariableList();
+                    variables = value.getMarshalVariableList();
 
                     // check if any of those variables were actually set
                     if (variables.size() > 0 || shouldMarshalEncoding(value)) {
@@ -380,7 +380,7 @@ private void userCommon(IRubyObject value, CacheEntry entry) throws IOException
 
         List<Variable<Object>> variables = null;
         if (marshaled.hasVariables()) {
-            variables = marshaled.getVariableList();
+            variables = marshaled.getMarshalVariableList();
             if (variables.size() > 0) {
                 write(TYPE_IVAR);
             } else {
diff --git a/spec/ruby/core/marshal/dump_spec.rb b/spec/ruby/core/marshal/dump_spec.rb
@@ -458,6 +458,12 @@ def obj.foo; end
       obj = MarshalSpec::BasicObjectSubWithRespondToFalse.new
       Marshal.dump(obj).should == "\x04\bo:2MarshalSpec::BasicObjectSubWithRespondToFalse\x00"
     end
+
+    it "dumps without marshaling any attached finalizer" do
+      obj = Object.new
+      ObjectSpace.define_finalizer(obj, &:itself)
+      Marshal.load(Marshal.dump(obj)).class.should == Object
+    end
   end
 
   describe "with a Range" do

Original file line number	Diff line number	Diff line change
`@@ -1269,7 +1269,7 @@ public void marshalTo(Ruby runtime, Object obj, RubyClass type, MarshalStream ma`
`1269`	`1269`	`IRubyObject object = (IRubyObject) obj;`
`1270`	`1270`
`1271`	`1271`	`marshalStream.registerLinkTarget(object);`
`1272`		`- marshalStream.dumpVariables(object.getVariableList());`
	`1272`	`+ marshalStream.dumpVariables(object.getMarshalVariableList());`
`1273`	`1273`	`}`
`1274`	`1274`
`1275`	`1275`	`@Override`