gh-106287: do not write objects after encountering an unmarshalling error

Writing out an object may involve a slot lookup, which is not safe to do with
an exception raised. In debug mode an assertion failure will occur if this
happens. To avoid assertion failures, and possibly more subtle problems the
assertion is there to prevent, skip attempts to write an object after an error.

Note the unmarshal writing code won't raise an exception itself, however the
set writing code calls back into the top-level unmarshal function (see
gh-78903), which will check for failures and raises an exception. Once that
happens it is not safe to continue writing further objects.

Note also that some data may still be written even after an error occurred and
an exception has been raised: code objects write objects and longs, and the
latter will be written unconditionally, even if the former fails. This
shouldn't cause a problem as it is documented that "garbage data will also be
written to the file" in such cases, and the top-level function will handle and
report the error appropriately.

Add a test case to cover the various different object types which could trigger
such failures.

Author: duaneg

Lib/test/test_marshal.py

@@ -408,6 +408,25 @@ def test_deterministic_sets(self):
                     _, dump_1, _ = assert_python_ok(*args, PYTHONHASHSEED="1")
                     self.assertEqual(dump_0, dump_1)
 
+    def test_unmarshallable(self):
+        # gh-106287
+        fset = frozenset([int])
+        code = compile("a = 1", "<string>", "exec")
+        code = code.replace(co_consts=(1, fset, None))
+        cases = (('tuple', (fset,)),
+                 ('list', [fset]),
+                 ('set', fset),
+                 ('dict key', {fset: 'x'}),
+                 ('dict value', {'x': fset}),
+                 ('dict key & value', {fset: fset}),
+                 ('dict set key & buffer', {fset: fset}),
+                 ('slice', slice(fset, fset)),
+                 ('code', code))
+        for name, arg in cases:
+            with self.subTest(name, arg=arg):
+                with self.assertRaisesRegex(ValueError, "unmarshallable object"):
+                    marshal.dumps((arg, memoryview(b'')))
+
 LARGE_SIZE = 2**31
 pointer_size = 8 if sys.maxsize > 0xFFFFFFFF else 4
 

Python/marshal.c

@@ -431,6 +431,9 @@ w_object(PyObject *v, WFILE *p)
 {
     char flag = '\0';
 
+    if (p->error != WFERR_OK)
+        return;
+
     p->depth++;
 
     if (p->depth > MAX_MARSHAL_STACK_DEPTH) {
@@ -750,6 +753,10 @@ PyMarshal_WriteLongToFile(long x, FILE *fp, int version)
     w_flush(&wf);
 }
 
+/* TODO: Contrary to its documentation, this function does NOT set an error on
+ * failure. It is not used internally except from test code, where this doesn't
+ * matter, but this discrepancy might cause problems for any external code
+ * using it. */
 void
 PyMarshal_WriteObjectToFile(PyObject *x, FILE *fp, int version)
 {