Fix crashes when trying to pickle CLR exceptions.

filmor · filmor · commit 82fdc393fcfc · 2016-11-08T15:06:20.000+01:00
The "args" slot of BaseException was not filled, instead we provided the
args through our __getattr__ implementation. This fails in BaseException_reduce
which depends on "args" being not NULL.

We fix this by explicitly setting the "args" slot on all CLR exception objects
on creation. This also makes tp_repr obsolete.
diff --git a/src/runtime/clrobject.cs b/src/runtime/clrobject.cs
@@ -30,6 +30,9 @@ internal CLRObject(Object ob, IntPtr tp) : base()
             this.pyHandle = py;
             this.gcHandle = gc;
             inst = ob;
+
+            // Fix the BaseException args slot if wrapping a CLR exception
+            Exceptions.SetArgs(py);
         }
 
 
diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs
@@ -62,30 +62,6 @@ internal static Exception ToException(IntPtr ob)
             return Runtime.PyUnicode_FromString(message);
         }
 
-        //====================================================================
-        // Exception __repr__ implementation.
-        //====================================================================
-
-        public static IntPtr tp_repr(IntPtr ob)
-        {
-            Exception e = ToException(ob);
-            if (e == null)
-            {
-                return Exceptions.RaiseTypeError("invalid object");
-            }
-            string name = e.GetType().Name;
-            string message;
-            if (e.Message != String.Empty)
-            {
-                message = String.Format("{0}('{1}',)", name, e.Message);
-            }
-            else
-            {
-                message = String.Format("{0}()", name);
-            }
-            return Runtime.PyUnicode_FromString(message);
-        }
-
         //====================================================================
         // Exceptions __getattribute__ implementation.
         // handles Python's args and message attributes
@@ -198,6 +174,36 @@ internal static void Shutdown()
             }
         }
 
+        /// <summary>
+        /// Set the 'args' slot on a python exception object that wraps
+        /// a CLR exception. This is needed for pickling CLR exceptions as
+        /// BaseException_reduce will only check the slots, bypassing the
+        /// __getattr__ implementation, and thus dereferencing a NULL
+        /// pointer.
+        /// </summary>
+        /// <param name="e">A CLR exception</param>
+        /// <param name="ob">The python object wrapping </param>
+        internal static void SetArgs(IntPtr ob)
+        {
+            var e = ExceptionClassObject.ToException(ob);
+            if (e == null)
+                return;
+
+            IntPtr args;
+            if (e.Message != String.Empty)
+            {
+                args = Runtime.PyTuple_New(1);
+                IntPtr msg = Runtime.PyUnicode_FromString(e.Message);
+                Runtime.PyTuple_SetItem(args, 0, msg);
+            }
+            else
+            {
+                args = Runtime.PyTuple_New(0);
+            }
+
+            Marshal.WriteIntPtr(ob, ExceptionOffset.args, args);
+        }
+
         /// <summary>
         ///  Shortcut for (pointer == NULL) -> throw PythonException
         /// </summary>
diff --git a/src/tests/test_exceptions.py b/src/tests/test_exceptions.py
@@ -6,11 +6,6 @@
     unicode = str
 
 
-# Note: all of these tests are known to fail because Python currently
-# doesn't allow new-style classes to be used as exceptions. I'm leaving
-# the tests in place in to document 'how it ought to work' in the hopes
-# that they'll all pass one day...
-
 class ExceptionTests(unittest.TestCase):
     """Test exception support."""
 
@@ -336,6 +331,16 @@ def testExceptionIsInstanceOfSystemObject(self):
         else:
             self.assertFalse(isinstance(o, Object))
 
+    def testPicklingExceptions(self):
+        from System import Exception
+        import pickle
+
+        exc = Exception("test")
+        dumped = pickle.dumps(exc)
+        loaded = pickle.loads(dumped)
+
+        self.assertEqual(repr(exc), repr(loaded))
+
 
 def test_suite():
     return unittest.makeSuite(ExceptionTests)

Original file line number	Diff line number	Diff line change
`@@ -30,6 +30,9 @@ internal CLRObject(Object ob, IntPtr tp) : base()`
`30`	`30`	`this.pyHandle = py;`
`31`	`31`	`this.gcHandle = gc;`
`32`	`32`	`inst = ob;`
	`33`	`+`
	`34`	`+ // Fix the BaseException args slot if wrapping a CLR exception`
	`35`	`+ Exceptions.SetArgs(py);`
`33`	`36`	`}`
`34`	`37`
`35`	`38`