Bug 1250: support for easier serialization

Adds MaybeSerialize<T> which handles the reload nicely. Shows how to use it in FieldObject; it's very minimal changes to FieldObject to add this support. This is simpler than the proof of concept code.
pythonnet · BadSingleton · Oct 19, 2020 · Oct 20, 2020 · Oct 9, 2020 · Oct 19, 2020
commit 9591cec7e6b4cdfa52ed7c35b8ce7687acecf38c
diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj
@@ -116,6 +116,7 @@
     <Compile Include="interfaces.cs" />
     <Compile Include="interop.cs" />
     <Compile Include="iterator.cs" />
+    <Compile Include="maybeserialize.cs" />
     <Compile Include="managedtype.cs" />
     <Compile Include="metatype.cs" />
     <Compile Include="methodbinder.cs" />

diff --git a/src/runtime/fieldobject.cs b/src/runtime/fieldobject.cs
@@ -12,97 +12,11 @@ namespace Python.Runtime
     [Serializable]
     internal class FieldObject : ExtensionType
     {
-        [Serializable]
-        private struct SerializedFieldInfo : ISerializable
-        {
-            // The field if we can find it. Otherwise null.
-            private FieldInfo m_info;
-
-            // The name of the field if the field is missing. Otherwise null.
-            private string m_name;
-
-            public SerializedFieldInfo(FieldInfo info)
-            {
-                if (info == null)
-                {
-                    throw new System.ArgumentNullException("null FieldInfo");
-                }
-                m_info = info;
-                m_name = null;
-            }
-
-            public FieldInfo Value
-            {
-                get
-                {
-                    if (m_info == null)
-                    {
-                        throw new SerializationException($".NET field {m_name} was renamed or removed during domain reload");
-                    }
-                    return m_info;
-                }
-            }
-
-            public string Name
-            {
-                get
-                {
-                    if (m_info == null)
-                    {
-                        return $"(missing {m_name})";
-                    }
-                    else
-                    {
-                        return m_info.Name;
-                    }
-                }
-            }
-
-            public void GetObjectData(SerializationInfo info, StreamingContext context)
-            {
-                if (m_info == null)
-                {
-                    info.AddValue("n", m_name);
-                }
-                else
-                {
-                    // Serialize in a silly way. TODO optimize.
-                    var formatter = new BinaryFormatter();
-                    using (var ms = new MemoryStream())
-                    {
-                        formatter.Serialize(ms, m_info);
-                        info.AddValue("i", ms.ToArray());
-                    }
-
-                    // Also save the name in case the info doesn't deserialize
-                    info.AddValue("n", m_info.ToString());
-                }
-            }
-
-            private SerializedFieldInfo(SerializationInfo info, StreamingContext context)
-            {
-                try
-                {
-                    var serialized = (byte[])info.GetValue("i", typeof(byte[]));
-                    var formatter = new BinaryFormatter();
-                    using (var ms = new MemoryStream(serialized))
-                    {
-                        m_info = (FieldInfo)formatter.Deserialize(ms);
-                    }
-                }
-                catch (SerializationException _)
-                {
-                    m_info = null;
-                }
-                m_name = (m_info != null) ? null : info.GetString("n");
-            }
-        }
-
-        private SerializedFieldInfo m_info;
+        private MaybeSerialize<FieldInfo> m_info;
 
         public FieldObject(FieldInfo info)
         {
-            m_info = new SerializedFieldInfo(info);
+            m_info = new MaybeSerialize<FieldInfo>(info);
         }
 
         /// <summary>
@@ -260,7 +174,7 @@ int tp_descr_set(IntPtr ob, IntPtr val)
         public static IntPtr tp_repr(IntPtr ob)
         {
             var self = (FieldObject)GetManagedObject(ob);
-            return Runtime.PyString_FromString($"<field '{self.m_info.Name}'>");
+            return Runtime.PyString_FromString($"<field '{self.m_info}'>");
         }
     }
 }
diff --git a/src/runtime/maybeserialize.cs b/src/runtime/maybeserialize.cs
@@ -0,0 +1,132 @@
+using System;
+using System.Reflection;
+using System.Runtime.Serialization;
+using System.Runtime.Serialization.Formatters.Binary;
+using System.IO;
+
+namespace Python.Runtime
+{
+    /// <summary>
+    /// A MaybeSerialize&lt;T&gt; delays errors from serialization and
+    /// deserialization until the item is used.
+    ///
+    /// Python for .NET uses this in the C# reloading architecture.
+    /// If e.g. a class member was renamed when reloading, references to the
+    /// old field will be invalid, but the rest of the system will still work.
+    /// Code that tries to use the old field will receive an exception.
+    ///
+    /// Assumption: the item being wrapped by MaybeSerialize will never be null.
+    /// </summary>
+    [Serializable]
+    internal struct MaybeSerialize<T> : ISerializable where T : class
+    {
+        /// <summary>
+        /// The item being wrapped.
+        ///
+        /// If this is null, that means we failed to serialize or deserialize it.
+        /// </summary>
+        private T m_item;
+
+        /// <summary>
+        /// A string useful for debugging the error.
+        ///
+        /// This is null if m_item deserialized properly.
+        /// Otherwise, it will be derived off of m_item.ToString() when we
+        /// serialized.
+        /// </summary>
+        private string m_name;
+
+        /// <summary>
+        /// Store an item in such a way that it can be deserialized.
+        ///
+        /// It must not be null.
+        /// </summary>
+        public MaybeSerialize(T item)
+        {
+            if (item == null)
+            {
+                throw new System.ArgumentNullException("Trying to store a null");
+            }
+            m_item = item;
+            m_name = null;
+        }
+
+        /// <summary>
+        /// Get the underlying deserialized value, or throw an exception
+        /// if deserialiation failed.
+        /// </summary>
+        public T Value
+        {
+            get
+            {
+                if (m_item == null)
+                {
+                    throw new SerializationException($"The .NET object underlying {m_name} no longer exists");
+                }
+                return m_item;
+            }
+        }
+
+        /// <summary>
+        /// Get a printable name.
+        /// </summary>
+        public string ToString()
+        {
+            if (m_item == null)
+            {
+                return $"(missing {m_name})";
+            }
+            else
+            {
+                return m_item.ToString();
+            }
+        }
+
+        /// <summary>
+        /// Implements ISerializable
+        /// </summary>
+        public void GetObjectData(SerializationInfo info, StreamingContext context)
+        {
+            if (m_item == null)
+            {
+                // Save the name; this failed to reload in a previous
+                // generation but we still need to remember what it was.
+                info.AddValue("n", m_name);
+            }
+            else
+            {
+                // Try to save the item. If it fails, too bad.
+                try
+                {
+                    info.AddValue("i", m_item);
+                }
+                catch(SerializationException _)
+                {
+                }
+
+                // Also save the name in case the item doesn't deserialize
+                info.AddValue("n", m_item.ToString());
+            }
+        }
+
+        /// <summary>
+        /// Implements ISerializable
+        /// </summary>
+        private MaybeSerialize(SerializationInfo info, StreamingContext context)
+        {
+            try
+            {
+                // Try to deserialize the item. It might fail, or it might
+                // have already failed so there just isn't an "i" to find.
+                m_item = (T)info.GetValue("i", typeof(T));
+                m_name = null;
+            }
+            catch (SerializationException _)
+            {
+                // Getting the item failed, so get the name.
+                m_item = null;
+                m_name = info.GetString("n");
+            }
+        }
+    }
+}