Skip to content

Fix exception pickling #286

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Nov 10, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix crashes when trying to pickle CLR exceptions.
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.
  • Loading branch information
filmor committed Nov 8, 2016
commit 82fdc393fcfc1c20df297578f37f513e2448c2b8
3 changes: 3 additions & 0 deletions src/runtime/clrobject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down
54 changes: 30 additions & 24 deletions src/runtime/exceptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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>
Expand Down
15 changes: 10 additions & 5 deletions src/tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""

Expand Down Expand Up @@ -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)
Expand Down