improve deserialization resolution of ref, out and in parameters

Also add a test
pythonnet · lostmsu · Jan 7, 2021 · Nov 9, 2020 · Nov 9, 2020 · Nov 18, 2020
commit 4d2d05b34b6890c96b661dad89ca6824b73902d0
diff --git a/src/domain_tests/TestRunner.cs b/src/domain_tests/TestRunner.cs
@@ -771,6 +771,84 @@ def after_reload():
     print(bar.__repr__())
                     ",
             },
+
+            new TestCase
+            {
+                Name = "out_to_ref_param",
+                DotNetBefore = @"
+                    namespace TestNamespace
+                    {
+
+                        [System.Serializable]
+                        public class Data
+                        {
+                            public int num = -1;
+                        }
+
+                        [System.Serializable]
+                        public class Cls 
+                        {
+                            public static void MyFn (out Data a)
+                            {
+                                a = new Data();
+                                a.num = 9001;
+                            }
+                        }
+                    }",
+                DotNetAfter = @"
+                    namespace TestNamespace
+                    {
+
+                        [System.Serializable]
+                        public class Data
+                        {
+                            public int num = -1;
+                        }
+
+                        [System.Serializable]
+                        public class Cls
+                        {
+                            public static void MyFn (ref Data a)
+                            {
+                                a.num = 7;
+                            }
+                        }
+                    }",
+                PythonCode = @"
+import clr
+import sys
+clr.AddReference('DomainTests')
+import TestNamespace
+import System
+
+def before_reload():
+
+    foo = TestNamespace.Data()
+    bar = TestNamespace.Cls.MyFn(foo)
+    assert bar.num == 9001
+    # foo shouldn't have changed.
+    assert foo.num == -1
+
+
+def after_reload():
+
+    try:
+        # Now that the function takes a ref type, we must pass a valid object.
+        bar = TestNamespace.Cls.MyFn(None)
+    except System.NullReferenceException as e:
+        print('caught expected exception')
+    else:
+        raise AssertionError('failed to raise')
+
+    foo = TestNamespace.Data()
+    bar = TestNamespace.Cls.MyFn(foo)
+    # foo should have changed
+    assert foo.num == 7
+    assert bar.num == 7
+    # Pythonnet also returns a new object with `ref`-quialified parameters
+    assert foo is not bar
+                    ",
+            },
         };
 
         /// <summary>

diff --git a/src/domain_tests/test_domain_reload.py b/src/domain_tests/test_domain_reload.py
@@ -59,4 +59,7 @@ def test_rename_event():
 
 @pytest.mark.xfail(reason="newly instanced object uses PyType_GenericAlloc")
 def test_construct_removed_class():
-    _run_test("construct_removed_class")
+    _run_test("construct_removed_class")
+
+def test_out_to_ref_param():
+    _run_test("out_to_ref_param")
diff --git a/src/runtime/StateSerialization/MaybeMethodBase.cs b/src/runtime/StateSerialization/MaybeMethodBase.cs
@@ -1,14 +1,50 @@
 using System;
 using System.Reflection;
 using System.Runtime.Serialization;
-using System.Runtime.Serialization.Formatters.Binary;
-using System.IO;
+using System.Linq;
 
 namespace Python.Runtime
 {
     [Serializable]
     internal struct MaybeMethodBase<T> : ISerializable where T: MethodBase
     {
+        [Serializable]
+        struct ParameterHelper : IEquatable<ParameterInfo>
+        {
+            public enum TypeModifier
+            {
+                None,
+                In,
+                Out,
+                Ref
+            }
+            public readonly string Name;
+            public readonly TypeModifier Modifier;
+
+            public ParameterHelper(ParameterInfo tp)
+            {
+                Name = tp.ParameterType.AssemblyQualifiedName;
+                Modifier = TypeModifier.None;
+
+                if (tp.IsIn)
+                {
+                    Modifier = TypeModifier.In;
+                }
+                else if (tp.IsOut)
+                {
+                    Modifier = TypeModifier.Out;
+                }
+                else if (tp.ParameterType.IsByRef)
+                {
+                    Modifier = TypeModifier.Ref;
+                }
+            }
+
+            public bool Equals(ParameterInfo other)
+            {
+                return this.Equals(new ParameterHelper(other));
+            }
+        }
         public static implicit operator MaybeMethodBase<T> (T ob) => new MaybeMethodBase<T>(ob);
 
         string name;
@@ -64,11 +100,11 @@ internal MaybeMethodBase(SerializationInfo serializationInfo, StreamingContext c
                 var tp = Type.GetType(serializationInfo.GetString("t"));
                 // Get the method's parameters types
                 var field_name = serializationInfo.GetString("f");
-                var param = (string[])serializationInfo.GetValue("p", typeof(string[]));
+                var param = (ParameterHelper[])serializationInfo.GetValue("p", typeof(ParameterHelper[]));
                 Type[] types = new Type[param.Length];
                 for (int i = 0; i < param.Length; i++)
                 {
-                    types[i] = Type.GetType(param[i]);
+                    types[i] = Type.GetType(param[i].Name);
                 }
                 // Try to get the method
                 MethodBase mb = tp.GetMethod(field_name, ClassManager.BindingFlags, binder:null, types:types, modifiers:null);
@@ -81,7 +117,29 @@ internal MaybeMethodBase(SerializationInfo serializationInfo, StreamingContext c
                 // Do like in ClassManager.GetClassInfo
                 if(mb != null && ClassManager.ShouldBindMethod(mb))
                 {
-                    info = mb;
+                    // One more step: Changing:
+                    // void MyFn (ref int a)
+                    // to:
+                    // void MyFn (out int a)
+                    // will still find the fucntion correctly as, `in`, `out` and `ref`
+                    // are all represented as a reference type. Query the method we got
+                    // and validate the parameters
+                    bool matches = true;
+                    if (param.Length != 0)
+                    {
+                        foreach (var item in Enumerable.Zip(param, mb.GetParameters(), (x, y) => new {x, y}))
+                        {
+                            if (!item.x.Equals(item.y))
+                            {
+                                matches = false;
+                                break;
+                            }
+                        }
+                    }
+                    if (matches)
+                    {
+                        info = mb;
+                    }
                 }
             }
             catch (Exception e)
@@ -97,13 +155,8 @@ public void GetObjectData(SerializationInfo serializationInfo, StreamingContext
             {
                 serializationInfo.AddValue("f", info.Name);
                 serializationInfo.AddValue("t", info.ReflectedType.AssemblyQualifiedName);
-                var p = info.GetParameters();
-                string[] types = new string[p.Length];
-                for (int i = 0; i < p.Length; i++)
-                {
-                    types[i] = p[i].ParameterType.AssemblyQualifiedName;
-                }
-                serializationInfo.AddValue("p", types, typeof(string[]));
+                ParameterHelper[] parameters = (from p in info.GetParameters() select new ParameterHelper(p)).ToArray();
+                serializationInfo.AddValue("p", parameters, typeof(ParameterHelper[]));
             }
         }
     }

diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs
@@ -866,6 +866,19 @@ int IComparer<MaybeMethodBase>.Compare(MaybeMethodBase m1, MaybeMethodBase m2)
         {
             MethodBase me1 = m1.Valid ? m1.Value : null;
             MethodBase me2 = m2.Valid ? m2.Value : null;
+            if (me1 == null && me2 == null)
+            {
+                return 0;
+            }
+            else if (me1 == null)
+            {
+                return -1;
+            }
+            else if (me2 == null)
+            {
+                return 1;
+            }
+
             if (me1.DeclaringType != me2.DeclaringType)
             {
                 // m2's type derives from m1's type, favor m2