Skip to content

Commit 48f18f0

Browse files
authored
gh-129724: gh-156504: Make marshal output independent of reference counts (GH-159192)
The marshal writer decided whether an object can be referenced later (FLAG_REF) from its reference count, so the output for the same value depended on what else in the process held a reference to its parts, and differed between the default and the free-threaded builds' view of the same objects. This is the remaining cause of gh-129724 and the cause of gh-156504. FLAG_REF is now set exactly on the objects which are written more than once in the dump. The writer still makes one pass: it records the position of the type byte of each object it remembers and of each TYPE_REF, and when the dump is complete it sets the flags and renumbers the references. A reference count of one is only used to skip the hashtable. Interned strings are no longer flagged unconditionally. The reader is unchanged and the format version is not bumped: both the old and the new output are loaded by old and new readers. PyMarshal_WriteObjectToFile() now builds the data in memory and writes it with one fwrite(), since the flags are only known at the end. Nothing is written if the object cannot be marshalled.
1 parent f9132f1 commit 48f18f0

7 files changed

Lines changed: 788 additions & 124 deletions

File tree

‎Doc/c-api/marshal.rst‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ and :exc:`MemoryError` if memory allocation fails.
5454
Marshal a Python object, *value*, to *file*.
5555
*version* indicates the file format.
5656
57+
The marshalled representation of *value* is built in memory
58+
and then written to *file*.
59+
Nothing is written if *value* cannot be marshalled.
60+
5761
This function can fail, in which case it sets the error indicator.
5862
Use :c:func:`PyErr_Occurred` to check for that.
5963
@@ -68,6 +72,11 @@ and :exc:`MemoryError` if memory allocation fails.
6872
It can use :c:func:`PyMarshal_WriteObjectToString` and write the
6973
result itself, or validate what was written.
7074
75+
.. versionchanged:: next
76+
Previously, the data was written to *file* as it was produced,
77+
using a buffer of constant size,
78+
and incomplete data was written if *value* could not be marshalled.
79+
7180
.. c:function:: PyObject* PyMarshal_WriteObjectToString(PyObject *value, int version)
7281
7382
Return a bytes object containing the marshalled representation of *value*.

‎Lib/test/test_capi/test_marshal.py‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,13 +121,39 @@ def test_write_object_to_file(self):
121121
write_object_to_file(obj, filename, version)
122122
data = read_file(filename)
123123
self.assertEqual(marshal.loads(data), obj)
124+
self.assertEqual(data, marshal.dumps(obj, version))
125+
126+
# Larger than a stdio buffer, with references to objects
127+
# written long before.
128+
shared = (float('1.5'), b'bytes' * 10)
129+
obj = [shared, 'long line ' * 10_000, shared, [shared] * 10_000]
130+
write_object_to_file(obj, filename, version)
131+
data = read_file(filename)
132+
self.assertEqual(data, marshal.dumps(obj, version))
133+
obj2 = marshal.loads(data)
134+
self.assertEqual(obj2, obj)
135+
if version >= 3:
136+
self.assertIs(obj2[2], obj2[0])
137+
self.assertIs(obj2[3][-1], obj2[0])
124138

125139
with self.assertRaises(SystemError):
126140
write_object_to_file(NULL, filename, version)
127141

128142
with self.assertRaisesRegex(ValueError, 'cannot marshal object objects'):
129143
write_object_to_file(UNMARSHALLABLE, filename, version)
130144

145+
# Nothing is written if the object cannot be marshalled,
146+
# even if the error is only detected after more data than
147+
# fits in a stdio buffer.
148+
for obj in (UNMARSHALLABLE,
149+
[1, 'abc', UNMARSHALLABLE],
150+
[shared, 'long line ' * 10_000, shared,
151+
UNMARSHALLABLE]):
152+
with self.assertRaisesRegex(ValueError,
153+
'cannot marshal object objects'):
154+
write_object_to_file(obj, filename, version)
155+
self.assertEqual(read_file(filename), b'')
156+
131157
def test_read_short_from_file(self):
132158
# Test PyMarshal_ReadShortFromFile()
133159
read_short_from_file = _testcapi.pymarshal_read_short_from_file

0 commit comments

Comments
 (0)