From 78939500b81313474d6030da0cb4f3fdc516553e Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Sun, 11 Oct 2026 17:23:28 +0000 Subject: [PATCH 1/2] gh-129724: Make marshal output independent of reference counts 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. --- Doc/c-api/marshal.rst | 9 + Lib/test/test_capi/test_marshal.py | 26 + Lib/test/test_marshal.py | 445 ++++++++++++++++++ ...-10-11-06-20-11.gh-issue-129724.Wq3nTb.rst | 4 + ...-10-11-06-00-00.gh-issue-129724.mRsh1x.rst | 9 + ...-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst | 3 + Programs/test_frozenmain.h | 28 +- Python/marshal.c | 390 ++++++++++----- 8 files changed, 790 insertions(+), 124 deletions(-) create mode 100644 Misc/NEWS.d/next/C_API/2026-10-11-06-20-11.gh-issue-129724.Wq3nTb.rst create mode 100644 Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst create mode 100644 Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst diff --git a/Doc/c-api/marshal.rst b/Doc/c-api/marshal.rst index 43fc0f2742d5d70..f8ff7f817ccf453 100644 --- a/Doc/c-api/marshal.rst +++ b/Doc/c-api/marshal.rst @@ -54,6 +54,10 @@ and :exc:`MemoryError` if memory allocation fails. Marshal a Python object, *value*, to *file*. *version* indicates the file format. + The marshalled representation of *value* is built in memory + and then written to *file*. + Nothing is written if *value* cannot be marshalled. + This function can fail, in which case it sets the error indicator. Use :c:func:`PyErr_Occurred` to check for that. @@ -68,6 +72,11 @@ and :exc:`MemoryError` if memory allocation fails. It can use :c:func:`PyMarshal_WriteObjectToString` and write the result itself, or validate what was written. + .. versionchanged:: next + Previously, the data was written to *file* as it was produced, + using a buffer of constant size, + and incomplete data was written if *value* could not be marshalled. + .. c:function:: PyObject* PyMarshal_WriteObjectToString(PyObject *value, int version) Return a bytes object containing the marshalled representation of *value*. diff --git a/Lib/test/test_capi/test_marshal.py b/Lib/test/test_capi/test_marshal.py index 4680c45f32dd4cd..7b69d89f6a46ff2 100644 --- a/Lib/test/test_capi/test_marshal.py +++ b/Lib/test/test_capi/test_marshal.py @@ -121,6 +121,20 @@ def test_write_object_to_file(self): write_object_to_file(obj, filename, version) data = read_file(filename) self.assertEqual(marshal.loads(data), obj) + self.assertEqual(data, marshal.dumps(obj, version)) + + # Larger than a stdio buffer, with references to objects + # written long before. + shared = (float('1.5'), b'bytes' * 10) + obj = [shared, 'long line ' * 10_000, shared, [shared] * 10_000] + write_object_to_file(obj, filename, version) + data = read_file(filename) + self.assertEqual(data, marshal.dumps(obj, version)) + obj2 = marshal.loads(data) + self.assertEqual(obj2, obj) + if version >= 3: + self.assertIs(obj2[2], obj2[0]) + self.assertIs(obj2[3][-1], obj2[0]) with self.assertRaises(SystemError): write_object_to_file(NULL, filename, version) @@ -128,6 +142,18 @@ def test_write_object_to_file(self): with self.assertRaisesRegex(ValueError, 'cannot marshal object objects'): write_object_to_file(UNMARSHALLABLE, filename, version) + # Nothing is written if the object cannot be marshalled, + # even if the error is only detected after more data than + # fits in a stdio buffer. + for obj in (UNMARSHALLABLE, + [1, 'abc', UNMARSHALLABLE], + [shared, 'long line ' * 10_000, shared, + UNMARSHALLABLE]): + with self.assertRaisesRegex(ValueError, + 'cannot marshal object objects'): + write_object_to_file(obj, filename, version) + self.assertEqual(read_file(filename), b'') + def test_read_short_from_file(self): # Test PyMarshal_ReadShortFromFile() read_short_from_file = _testcapi.pymarshal_read_short_from_file diff --git a/Lib/test/test_marshal.py b/Lib/test/test_marshal.py index 5b4d02c232f4a64..579c72306a4a2bd 100644 --- a/Lib/test/test_marshal.py +++ b/Lib/test/test_marshal.py @@ -1,10 +1,13 @@ from test import support from test.support import is_apple_mobile, os_helper, requires_debug_ranges, is_emscripten +from test.support import threading_helper from test.support.script_helper import assert_python_ok import array import io import marshal +import struct import sys +import threading import unittest import os import types @@ -865,5 +868,447 @@ def test_slice(self): marshal.dumps(obj, version) +def reachable_objects(obj): + """Return a list of all objects marshalled as a part of obj.""" + result = [] + seen = set() + def visit(x): + if id(x) in seen: + return + seen.add(id(x)) + result.append(x) + if isinstance(x, (tuple, list, set, frozenset)): + for item in x: + visit(item) + elif isinstance(x, (dict, frozendict)): + for key, value in x.items(): + visit(key) + visit(value) + elif isinstance(x, slice): + visit(x.start) + visit(x.stop) + visit(x.step) + elif isinstance(x, types.CodeType): + for name in ('co_code', 'co_consts', 'co_names', 'co_varnames', + 'co_freevars', 'co_cellvars', 'co_filename', + 'co_name', 'co_qualname', 'co_linetable', + 'co_exceptiontable'): + visit(getattr(x, name)) + visit(obj) + return result + + +class ReferenceTestCase(unittest.TestCase): + # The output must only depend on the marshalled objects, not on + # the number of references to them in the rest of the process: + # an object is written with FLAG_REF if and only if it occurs more + # than once in the marshalled object graph. + + FLAG_REF = 0x80 + # The tests comparing the output with literal data use this version. + VERSION = 6 + + @staticmethod + def make_sample(version=marshal.version): + # Create new objects on every call, so that nothing else + # refers to them. + bigint = int('1234567890' * 4) + flt = float('1.25') + cplx = complex('1.5+2.5j') + data = bytes(range(16)) + text = ' '.join(['spam', 'eggs']) + nonascii = '\u20ac' + text + inner = (bigint, flt, cplx, data, text, nonascii) + sample = [ + inner, + (flt, (data, (bigint,))), + [text, [cplx]], + {flt: inner, text: [data]}, + {(bigint, 1), (data, 2)}, + frozenset([(text, 1), (flt, 2), nonascii]), + (inner, bigint, inner), + ] + if version >= 5: + sample.append(slice(bigint, (flt, text), data)) + if version >= 6: + sample.append(frozendict({text: inner, 'key': (cplx,)})) + return sample + + def check_extra_references(self, obj, version): + expected = marshal.dumps(obj, version) + extra = reachable_objects(obj) + self.assertEqual(marshal.dumps(obj, version), expected) + extra2 = [list(extra), tuple(extra)] + self.assertEqual(marshal.dumps(obj, version), expected) + del extra, extra2 + self.assertEqual(marshal.dumps(obj, version), expected) + return expected + + def test_extra_references(self): + for version in range(marshal.version + 1): + with self.subTest(version=version): + sample = self.make_sample(version) + data = self.check_extra_references(sample, version) + # Equal graphs give equal output. + self.assertEqual(marshal.dumps(self.make_sample(version), + version), data) + self.assertEqual(marshal.loads(data), sample) + + def test_extra_references_code(self): + source = textwrap.dedent(""" + def f(a, b=(1, 2.5, 'spam eggs')): + x = {'ham', 12345678901234567890, b'bytes'} + def g(c): + return (a, b, c, x, (1, 2.5, 'spam eggs'), 6.25j) + return g + class C: + attr = (None, ..., 2**70) + def method(self): + return self.attr, 'spam eggs' + """) + for version in range(marshal.version + 1): + with self.subTest(version=version): + code = compile(source, '', 'exec') + data = self.check_extra_references(code, version) + self.assertEqual(marshal.loads(data), code) + + @support.cpython_only + def test_single_occurrence_is_not_flagged(self): + # No FLAG_REF for an object which occurs only once, even if it is + # immortal, interned or referenced from elsewhere. + version = self.VERSION + interned = sys.intern('marshal_interned_name') + referenced = self.make_sample(version) + keep = reachable_objects(referenced) + for obj, typecode in [ + (1, 'i'), + (2**100, 'l'), + (1.5, 'g'), + (1.5j, 'y'), + (b'', 's'), + (b'bytes', 's'), + ('', 'Z'), + ('a', 'Z'), + (interned, 'Z'), + (''.join(['\u20ac', 'uro']), 'u'), + (' '.join(['not', 'interned']), 'z'), + (sys.intern('\u20ac\u20ac'), 't'), + ((), ')'), + (frozenset(), '>'), + (referenced[0], ')'), + (referenced[2], '['), + (referenced[3], '{'), + (referenced[5], '>'), + (referenced[7], ':'), + (referenced[8], '}'), + (self.make_sample.__code__, 'c'), + ]: + with self.subTest(obj=obj): + data = marshal.dumps(obj, version) + self.assertEqual(data[0], ord(typecode)) + data = marshal.dumps(referenced, version) + self.assertEqual(marshal.dumps(self.make_sample(version), version), + data) + del keep + self.assertEqual(marshal.dumps(referenced, version), data) + + a = float('1.5') + b = float('2.5') + keep = [a, b, a, b] + data_a = b'g' + struct.pack('', 'eval') + self.assertEqual(code.co_names, ('marshal_name_once',)) + self.assertIs(code.co_names[0], sys.intern('marshal_name_once')) + data = marshal.dumps(code, version) + self.assertEqual(data.count(b'\x11marshal_name_once'), 1) + self.assertIn(b'Z\x11marshal_name_once', data) + new = marshal.loads(data) + self.assertEqual(new, code) + self.assertIs(new.co_names[0], code.co_names[0]) + + @support.cpython_only + def test_reference_index(self): + # Only the objects occurring more than once are numbered, in + # the order of their first occurrence. + a = float('1.5') + b = float('2.5') + c = float('3.5') + data_a = b'g' + struct.pack('', 'exec') + objects = [ + int('1234567890' * 4), float('1.25'), complex('1.5+2.5j'), + bytes(range(16)), ' '.join(['spam', 'eggs']), + '\u20ac' * 3, (float('1.25'), 'x'), [1, 2], {1: 2}, {1, 2}, + frozenset([float('1.25'), 2]), code, code.co_code, + ] + for version in range(3, marshal.version + 1): + if version >= 5: + objects.append(slice(1, float('1.25'))) + if version >= 6: + objects.append(frozendict(x=[1])) + for obj in objects: + with self.subTest(version=version, obj=obj): + for sample in ([obj, obj], (obj, [obj], obj), + {'a': obj, 'b': (obj,)}, + [obj] * 300): + new = marshal.loads(marshal.dumps(sample, version)) + self.assertEqual(new, sample) + new = reachable_objects(new) + self.assertEqual( + sum(type(x) is type(obj) and x == obj + for x in new), 1) + # All together, in a different order. + sample = [objects, list(reversed(objects)), tuple(objects)] + new = marshal.loads(marshal.dumps(sample, version)) + self.assertEqual(new, sample) + for x, y, z in zip(new[0], reversed(new[1]), new[2]): + self.assertIs(x, y) + self.assertIs(x, z) + + def test_shared_objects_old_versions(self): + sample = self.make_sample(2) + for version in range(3): + with self.subTest(version=version): + data = marshal.dumps([sample, sample], version) + for obj in reachable_objects(sample): + self.assertFalse( + marshal.dumps(obj, version)[0] & self.FLAG_REF) + new = marshal.loads(data) + self.assertEqual(new, [sample, sample]) + self.assertIsNot(new[0], new[1]) + + def test_nan_in_set(self): + # Distinct NaNs are distinct elements of a set, and they have + # the same marshalled representation. + for kind in set, frozenset: + for version in range(marshal.version + 1): + with self.subTest(kind=kind, version=version): + nan1 = float('nan') + nan2 = float('nan') + nan3 = float('nan') + s = kind([nan1, 1.5, nan2, nan3]) + for sample in (s, [s], [nan1, s], [s, nan2], + [nan3, s, nan1], [nan1, nan2, nan3, s], + [s, s], (nan2, [s, nan2], s)): + data = marshal.dumps(sample, version) + self.assertEqual(marshal.dumps(sample, version), data) + new = marshal.loads(data) + self.check_same_structure(new, sample, version >= 3) + + def check_same_structure(self, new, sample, shared): + # Both object graphs have the same values and, if shared is true, + # share the same objects. Elements of sets are matched in any + # order. + def shape(obj): + seen = {} + result = [] + def number(x): + if shared: + return seen.setdefault(id(x), len(seen)) + def known(x): + if shared: + return seen.get(id(x), -1) + def visit(x): + if isinstance(x, float): + result.append((repr(x), number(x))) + elif isinstance(x, (set, frozenset)): + # Elements are only numbered if they occur elsewhere. + result.append((type(x), number(x), + sorted((repr(y), known(y)) for y in x))) + else: + result.append((type(x), len(x))) + for y in x: + visit(y) + visit(obj) + # Again, with all objects numbered. + result.clear() + visit(obj) + return result + self.assertEqual(shape(new), shape(sample)) + + def test_shared_recursion(self): + # A tuple which is referenced several times is not mistaken + # for a recursive one, and a recursive one is still detected. + a = (float('1.25'),) + b = (a, a) + sample = [a, b, (b, a), b] + keep = reachable_objects(sample) + new = marshal.loads(marshal.dumps(sample)) + self.assertEqual(new, sample) + self.assertIs(new[1][0], new[0]) + self.assertIs(new[2][0], new[1]) + + lst = [] + rec = (a, b, lst) + lst.append(rec) + for sample in (rec, [a, b, rec], (rec, rec)): + with self.assertRaisesRegex( + ValueError, 'cannot marshal recursion tuple objects'): + marshal.dumps(sample) + lst.clear() + + def test_dump_to_file(self): + # dump() writes the same data as dumps(), also if the data is larger + # than a buffer and contains references to objects written before. + sample = self.make_sample() + sample = [sample, 'x' * 100_000, sample, [sample[0]] * 10_000] + self.addCleanup(os_helper.unlink, os_helper.TESTFN) + for version in range(marshal.version + 1): + with self.subTest(version=version): + if version < 6: + sample[0] = sample[2] = self.make_sample(version) + data = marshal.dumps(sample, version) + with open(os_helper.TESTFN, 'wb') as f: + marshal.dump(sample, f, version) + with open(os_helper.TESTFN, 'rb') as f: + self.assertEqual(f.read(), data) + with open(os_helper.TESTFN, 'rb') as f: + self.assertEqual(marshal.load(f), sample) + f = io.BytesIO() + marshal.dump(sample, f, version) + self.assertEqual(f.getvalue(), data) + + @threading_helper.requires_working_threading() + def test_threads(self): + # The output does not depend on other threads using the same objects. + code = compile(textwrap.dedent(""" + def f(a, b=(1, 2.5, 'spam eggs')): + return [a, b, {'ham', 12345678901234567890}, b'bytes'] + """), '', 'exec') + sample = [self.make_sample(), code] + expected = marshal.dumps(sample) + results = [] + def work(): + for i in range(20): + keep = reachable_objects(sample) + results.append(marshal.dumps(sample)) + threads = [threading.Thread(target=work) for i in range(4)] + with threading_helper.start_threads(threads): + pass + self.assertEqual(len(results), 80) + self.assertEqual(set(results), {expected}) + + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/C_API/2026-10-11-06-20-11.gh-issue-129724.Wq3nTb.rst b/Misc/NEWS.d/next/C_API/2026-10-11-06-20-11.gh-issue-129724.Wq3nTb.rst new file mode 100644 index 000000000000000..5a6f6476e6dcae4 --- /dev/null +++ b/Misc/NEWS.d/next/C_API/2026-10-11-06-20-11.gh-issue-129724.Wq3nTb.rst @@ -0,0 +1,4 @@ +:c:func:`PyMarshal_WriteObjectToFile` now builds the marshalled data in +memory before writing it to the file, instead of writing it piece by piece +using a buffer of constant size. Nothing is written any more if the object +cannot be marshalled. diff --git a/Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst b/Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst new file mode 100644 index 000000000000000..301b146bd4cbc34 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst @@ -0,0 +1,9 @@ +The output of :func:`marshal.dumps` and :func:`marshal.dump`, and so the +content of :file:`.pyc` files, no longer depends on the reference counts of +the serialized objects: it is no longer changed by unrelated references to +them held elsewhere in the process. An object is now written in a form that +can be referenced later in the data if and only if it occurs more than once +in the serialized value; this also applies to interned strings, which were +always written in that form. The format is unchanged and the data can be +read by previous Python versions that support the format version, but it is +not byte-for-byte identical to the data written by them. diff --git a/Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst b/Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst new file mode 100644 index 000000000000000..272ecba61c676ff --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst @@ -0,0 +1,3 @@ +In the :term:`free-threaded build`, the output of :func:`marshal.dumps` and +:func:`marshal.dump`, and so the content of :file:`.pyc` files, no longer +depends on whether the serialized objects have been used by other threads. diff --git a/Programs/test_frozenmain.h b/Programs/test_frozenmain.h index c82b791439a3fa1..ebcb38b18dec90c 100644 --- a/Programs/test_frozenmain.h +++ b/Programs/test_frozenmain.h @@ -1,7 +1,7 @@ // Auto-generated by Programs/freeze_test_frozenmain.py unsigned char M_test_frozenmain[] = { - 227,0,0,0,0,0,0,0,0,0,0,0,0,9,0,0, - 0,0,0,0,0,243,188,0,0,0,128,0,0,0,90,0, + 99,0,0,0,0,0,0,0,0,0,0,0,0,9,0,0, + 0,0,0,0,0,115,188,0,0,0,128,0,0,0,90,0, 77,7,69,0,112,0,90,0,77,7,69,4,112,1,89,2, 31,0,78,1,50,1,0,0,0,0,0,0,29,0,89,2, 31,0,78,2,89,0,76,6,0,0,0,0,0,0,0,0, @@ -13,27 +13,27 @@ unsigned char M_test_frozenmain[] = { 89,2,31,0,78,4,89,6,12,0,78,5,89,5,89,6, 42,26,0,0,0,0,0,0,0,0,0,0,12,0,48,4, 50,1,0,0,0,0,0,0,29,0,71,26,0,0,9,0, - 28,0,77,7,33,0,41,7,233,0,0,0,0,122,18,70, + 28,0,77,7,33,0,41,7,105,0,0,0,0,122,18,70, 114,111,122,101,110,32,72,101,108,108,111,32,87,111,114,108, 100,122,8,115,121,115,46,97,114,103,118,218,6,99,111,110, 102,105,103,122,7,99,111,110,102,105,103,32,122,2,58,32, - 41,5,218,12,112,114,111,103,114,97,109,95,110,97,109,101, - 218,10,101,120,101,99,117,116,97,98,108,101,218,15,117,115, - 101,95,101,110,118,105,114,111,110,109,101,110,116,218,17,99, + 41,5,90,12,112,114,111,103,114,97,109,95,110,97,109,101, + 90,10,101,120,101,99,117,116,97,98,108,101,90,15,117,115, + 101,95,101,110,118,105,114,111,110,109,101,110,116,90,17,99, 111,110,102,105,103,117,114,101,95,99,95,115,116,100,105,111, - 218,14,98,117,102,102,101,114,101,100,95,115,116,100,105,111, - 41,7,218,3,115,121,115,218,17,95,116,101,115,116,105,110, - 116,101,114,110,97,108,99,97,112,105,218,5,112,114,105,110, - 116,218,4,97,114,103,118,218,11,103,101,116,95,99,111,110, - 102,105,103,115,114,3,0,0,0,218,3,107,101,121,169,0, - 243,0,0,0,0,218,18,116,101,115,116,95,102,114,111,122, + 90,14,98,117,102,102,101,114,101,100,95,115,116,100,105,111, + 41,7,90,3,115,121,115,90,17,95,116,101,115,116,105,110, + 116,101,114,110,97,108,99,97,112,105,90,5,112,114,105,110, + 116,90,4,97,114,103,118,90,11,103,101,116,95,99,111,110, + 102,105,103,115,114,0,0,0,0,90,3,107,101,121,41,0, + 243,0,0,0,0,90,18,116,101,115,116,95,102,114,111,122, 101,110,109,97,105,110,46,112,121,218,8,60,109,111,100,117, - 108,101,62,114,18,0,0,0,1,0,0,0,115,94,0,0, + 108,101,62,114,2,0,0,0,1,0,0,0,115,94,0,0, 0,241,3,1,1,1,243,8,0,1,11,219,0,24,225,0, 5,208,6,26,212,0,27,217,0,5,128,106,144,35,151,40, 145,40,212,0,27,216,9,26,215,9,38,210,9,38,211,9, 40,168,24,213,9,50,128,6,244,2,6,12,2,128,67,241, 14,0,5,10,136,71,144,67,144,53,152,2,152,54,160,35, 157,59,152,45,208,10,40,214,4,41,243,15,6,12,2,114, - 16,0,0,0, + 1,0,0,0, }; diff --git a/Python/marshal.c b/Python/marshal.c index b6a50627854eaf9..89399086b24e6e4 100644 --- a/Python/marshal.c +++ b/Python/marshal.c @@ -100,15 +100,45 @@ module marshal #define FLAG_REF '\x80' /* with a type, add obj to index */ +/* State of an object which can be the target of a TYPE_REF (version 3+), + see w_ref() and w_resolve_refs(). + + WFILE.hashtable maps the object to its index in WFILE.refs, shifted left + by W_ENTRY_SHIFT bits, plus the flags: + - W_ENTRY_ACTIVE: the object is being written (used to detect recursion, + see w_complete()); + - W_ENTRY_SHARED: the object has been written more than once. + + The item of WFILE.refs is the offset of the type byte of the object in + the output, shifted left by one bit, plus W_REF_SHARED if the object has + been written more than once. */ +#define W_ENTRY_ACTIVE ((uintptr_t)1) +#define W_ENTRY_SHARED ((uintptr_t)2) +#define W_ENTRY_SHIFT 2 +#define W_REF_SHARED ((size_t)1) + +/* Maximum number of items in WFILE.refs. TYPE_REF has a signed 32-bit + argument. */ +#define W_MAX_REFS Py_MIN((uintptr_t)0x7fffffff, UINTPTR_MAX >> W_ENTRY_SHIFT) + typedef struct { - FILE *fp; bool error; /* An exception has been raised. */ int depth; PyObject *str; char *ptr; const char *end; char *buf; - _Py_hashtable_t *hashtable; + _Py_hashtable_t *hashtable; /* object -> index in refs, flags */ + size_t *refs; /* In order of the first occurrence. */ + size_t refs_allocated; + unsigned char *refpos; /* Where the TYPE_REFs are in the + output, see w_add_refpos(). */ + size_t refpos_size; + size_t refpos_allocated; + size_t refpos_run; /* Index in refpos of the length of + the current run, or 0. */ + size_t refpos_end; /* Offset of the end of the last + TYPE_REF in the output. */ int version; int allow_code; } WFILE; @@ -118,45 +148,12 @@ typedef struct { *(p)->ptr++ = (c); \ } while(0) -/* Report a failure of the underlying file. An earlier error is not - overwritten. */ -static void -w_file_error(WFILE *p) -{ - int saved_errno = errno; - if (p->error) { - return; - } - p->error = true; - if (PyErr_CheckSignals()) { - /* The signal handler has raised an exception. */ - return; - } - errno = saved_errno; - PyErr_SetFromErrno(PyExc_OSError); -} - -static void -w_flush(WFILE *p) -{ - assert(p->fp != NULL); - size_t n = (size_t)(p->ptr - p->buf); - if (fwrite(p->buf, 1, n, p->fp) != n) { - w_file_error(p); - } - p->ptr = p->buf; -} - static int w_reserve(WFILE *p, Py_ssize_t needed) { Py_ssize_t pos, size, delta; if (p->ptr == NULL) return 0; /* An error already occurred */ - if (p->fp != NULL) { - w_flush(p); - return needed <= p->end - p->ptr; - } assert(p->str != NULL); pos = p->ptr - p->buf; size = PyBytes_GET_SIZE(p->str); @@ -191,23 +188,9 @@ w_string(const void *s, Py_ssize_t n, WFILE *p) if (!n || p->ptr == NULL) return; m = p->end - p->ptr; - if (p->fp != NULL) { - if (n <= m) { - memcpy(p->ptr, s, n); - p->ptr += n; - } - else { - w_flush(p); - if (fwrite(s, 1, n, p->fp) != (size_t)n) { - w_file_error(p); - } - } - } - else { - if (n <= m || w_reserve(p, n - m)) { - memcpy(p->ptr, s, n); - p->ptr += n; - } + if (n <= m || w_reserve(p, n - m)) { + memcpy(p->ptr, s, n); + p->ptr += n; } } @@ -227,6 +210,23 @@ w_long(long x, WFILE *p) w_byte((char)((x>>24) & 0xff), p); } +/* Read and write a 32-bit value in place. */ +static inline uint32_t +w_get_uint32(const char *ptr) +{ + const unsigned char *b = (const unsigned char *)ptr; + return b[0] | (b[1] << 8) | (b[2] << 16) | ((uint32_t)b[3] << 24); +} + +static inline void +w_put_uint32(char *ptr, uint32_t x) +{ + ptr[0] = (char)( x & 0xff); + ptr[1] = (char)((x>> 8) & 0xff); + ptr[2] = (char)((x>>16) & 0xff); + ptr[3] = (char)((x>>24) & 0xff); +} + #define SIZE32_MAX 0x7FFFFFFF #if SIZEOF_SIZE_T > 4 @@ -266,9 +266,10 @@ w_short_pstring(const void *s, Py_ssize_t n, WFILE *p) #define PyLong_MARSHAL_BASE ((short)1 << PyLong_MARSHAL_SHIFT) #define PyLong_MARSHAL_MASK (PyLong_MARSHAL_BASE - 1) -#define W_TYPE(t, p) do { \ - w_byte((t) | flag, (p)); \ -} while(0) +/* Write the type code of an object. It must be the first byte written for + an object passed to w_ref(): w_resolve_refs() adds FLAG_REF to that byte + if the object turns out to be referenced more than once. */ +#define W_TYPE(t, p) w_byte((t), (p)) static PyObject * _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code); @@ -316,7 +317,7 @@ _r_digits(32) #undef _r_digits static void -w_PyLong(const PyLongObject *ob, char flag, WFILE *p) +w_PyLong(const PyLongObject *ob, WFILE *p) { W_TYPE(TYPE_LONG, p); if (_PyLong_IsZero(ob)) { @@ -399,20 +400,116 @@ w_float_str(double v, WFILE *p) PyMem_Free(buf); } +/* Make room for one more item in a dynamic array of n items. + Return the array, which may have moved, or NULL on memory error. */ +static void * +w_array_grow(void *items, size_t n, size_t *allocated, size_t itemsize) +{ + if (n < *allocated) { + return items; + } + size_t new_allocated = *allocated ? *allocated * 2 : 16; + if (new_allocated > (size_t)PY_SSIZE_T_MAX / itemsize) { + return NULL; + } + items = PyMem_Realloc(items, new_allocated * itemsize); + if (items != NULL) { + *allocated = new_allocated; + } + return items; +} + +/* Remember that a TYPE_REF is being written at the current end of the + output. Return 0 on memory error. + + p->refpos lists the TYPE_REFs in the order of the output, as a sequence + of items which start with the byte: + - 1 to 254: a TYPE_REF follows that number of other bytes of output; + - 255, followed by a size_t: the same for a larger number of bytes; + - 0, followed by a byte n: n more TYPE_REFs directly follow the previous + one. + This takes one byte for most TYPE_REFs, and much less when the output + mostly consists of TYPE_REFs. */ static int -w_ref(PyObject *v, char *flag, WFILE *p) +w_add_refpos(WFILE *p) +{ + size_t pos = (size_t)(p->ptr - p->buf); + size_t gap = pos - p->refpos_end; + p->refpos_end = pos + 5; + if (gap == 0 && p->refpos_run != 0 && p->refpos[p->refpos_run] < 255) { + p->refpos[p->refpos_run]++; + return 1; + } + unsigned char *refpos = w_array_grow(p->refpos, + p->refpos_size + sizeof(size_t), + &p->refpos_allocated, 1); + if (refpos == NULL) { + PyErr_NoMemory(); + return 0; + } + p->refpos = refpos; + size_t size = p->refpos_size; + if (gap == 0) { + refpos[size++] = 0; + p->refpos_run = size; + refpos[size++] = 1; + } + else { + p->refpos_run = 0; + if (gap < 255) { + refpos[size++] = (unsigned char)gap; + } + else { + refpos[size++] = 255; + memcpy(refpos + size, &gap, sizeof(gap)); + size += sizeof(gap); + } + } + p->refpos_size = size; + return 1; +} + +/* Object references (version 3+). + + The reader keeps a list of the objects whose type byte has FLAG_REF, in + the order in which they start in the stream, and TYPE_REF refers to an + object by its index in that list. + + Whether an object is flagged must only depend on the object graph being + written, not on how many other references to the object happen to exist + in the process, otherwise the output is not reproducible. So an object + is flagged if and only if it is written more than once. That is not + known when the object is seen for the first time, so: + + - on the first occurrence, w_ref() records the offset of the type byte + (the next byte to be written, see W_TYPE) in p->refs and the object is + written without FLAG_REF; + - on a later occurrence, w_ref() marks the object as shared and writes + a TYPE_REF whose argument is provisional: the index of the object in + p->refs. The position of the TYPE_REF is recorded in p->refpos; + - once everything is written, w_resolve_refs() adds FLAG_REF to the + type byte of each shared object, numbers these objects in the order + of their first occurrence and fixes up the arguments of all + TYPE_REFs. + + Return 1 if the object has been dealt with (TYPE_REF written or error), + 0 if the caller has to write it. + */ +static int +w_ref(PyObject *v, WFILE *p) { _Py_hashtable_entry_t *entry; if (p->version < 3 || p->hashtable == NULL) return 0; /* not writing object references */ - /* If it has only one reference, it definitely isn't shared. - * But we use TYPE_REF always for interned string, to PYC file stable - * as possible. + /* An object with only one reference cannot occur twice in the object + * graph, so there is no need to remember it. This is only a shortcut: + * an object occurring twice has at least two references each time it is + * seen, so it is never skipped and the output does not depend on this + * test. */ - if (_PyObject_IsUniquelyReferenced(v) && - !(PyUnicode_CheckExact(v) && PyUnicode_CHECK_INTERNED(v))) { + if (_PyObject_IsUniquelyReferenced(v)) { return 0; } @@ -420,36 +517,57 @@ w_ref(PyObject *v, char *flag, WFILE *p) if (entry != NULL) { /* write the reference index to the stream */ uintptr_t w = (uintptr_t)entry->value; - if (w & 0x80000000LU) { + if (w & W_ENTRY_ACTIVE) { PyErr_Format(PyExc_ValueError, "cannot marshal recursion %T objects", v); goto err; } - /* we don't store "long" indices in the dict */ - assert(w <= 0x7fffffff); - w_byte(TYPE_REF, p); - w_long((int)w, p); + if (!(w & W_ENTRY_SHARED)) { + entry->value = (void *)(w | W_ENTRY_SHARED); + p->refs[w >> W_ENTRY_SHIFT] |= W_REF_SHARED; + } + w >>= W_ENTRY_SHIFT; + assert(w < p->hashtable->nentries); + if (p->end - p->ptr < 5 && !w_reserve(p, 5)) { + return 1; + } + if (!w_add_refpos(p)) { + goto err; + } + *p->ptr++ = TYPE_REF; + /* The index in the reader's reference list is not known yet. + Write the index in p->refs, w_resolve_refs() will replace it. */ + w_put_uint32(p->ptr, (uint32_t)w); + p->ptr += 4; return 1; } else { size_t w = p->hashtable->nentries; /* we don't support long indices */ - if (w >= 0x7fffffff) { + if (w >= W_MAX_REFS) { PyErr_SetString(PyExc_ValueError, "too many objects"); goto err; } + size_t *refs = w_array_grow(p->refs, w, &p->refs_allocated, + sizeof(*refs)); + if (refs == NULL) { + PyErr_NoMemory(); + goto err; + } + p->refs = refs; + refs[w] = (size_t)(p->ptr - p->buf) << 1; + uintptr_t value = (uintptr_t)w << W_ENTRY_SHIFT; // Corresponding code should call w_complete() after // writing the object. if (PyTuple_CheckExact(v) || PyCode_Check(v) || PySlice_Check(v) || PyFrozenDict_CheckExact(v)) { - w |= 0x80000000LU; + value |= W_ENTRY_ACTIVE; } if (_Py_hashtable_set(p->hashtable, Py_NewRef(v), - (void *)(uintptr_t)w) < 0) { + (void *)value) < 0) { Py_DECREF(v); PyErr_NoMemory(); goto err; } - *flag |= FLAG_REF; return 0; } err: @@ -457,6 +575,60 @@ w_ref(PyObject *v, char *flag, WFILE *p) return 1; } +/* Finish the output: flag the objects which are the target of a TYPE_REF + and replace the provisional argument of each TYPE_REF. See w_ref(). */ +static void +w_resolve_refs(WFILE *p) +{ + if (p->error || p->refpos == NULL) { + return; + } + assert(p->hashtable != NULL); + /* Replace the state of each object in p->refs with its index in the + reader's reference list (only used for the shared objects). */ + size_t nrefs = p->hashtable->nentries; + size_t *refs = p->refs; + uint32_t index = 0; + for (size_t i = 0; i < nrefs; i++) { + size_t shared = refs[i] & W_REF_SHARED; + char *type = p->buf + (refs[i] >> 1); + assert(!(*type & FLAG_REF)); + /* No branch on shared: it would not be predictable. */ + *type |= (char)(-(int)shared & FLAG_REF); + refs[i] = index; + index += (uint32_t)shared; +#ifdef Py_DEBUG + if (!shared) { + refs[i] = (size_t)-1; + } +#endif + } + const unsigned char *pos = p->refpos; + const unsigned char *end = pos + p->refpos_size; + char *ptr = p->buf; + while (pos < end) { + size_t gap = *pos++; + size_t count = 1; + if (gap == 0) { + count = *pos++; + } + else if (gap == 255) { + memcpy(&gap, pos, sizeof(gap)); + pos += sizeof(gap); + } + ptr += gap; + do { + assert(ptr + 5 <= p->ptr); + assert(*ptr == TYPE_REF); + uint32_t w = w_get_uint32(ptr + 1); + assert(w < nrefs); + assert(refs[w] < index); + w_put_uint32(ptr + 1, (uint32_t)refs[w]); + ptr += 5; + } while (--count); + } +} + static void w_complete(PyObject *v, WFILE *p) { @@ -473,19 +645,17 @@ w_complete(PyObject *v, WFILE *p) } assert(entry != NULL); uintptr_t w = (uintptr_t)entry->value; - assert(w & 0x80000000LU); - w &= ~0x80000000LU; - entry->value = (void *)(uintptr_t)w; + assert(w & W_ENTRY_ACTIVE); + w &= ~W_ENTRY_ACTIVE; + entry->value = (void *)w; } static void -w_complex_object(PyObject *v, char flag, WFILE *p); +w_complex_object(PyObject *v, WFILE *p); static void w_object(PyObject *v, WFILE *p) { - char flag = '\0'; - if (p->error) { return; } @@ -515,14 +685,14 @@ w_object(PyObject *v, WFILE *p) else if (v == Py_True) { w_byte(TYPE_TRUE, p); } - else if (!w_ref(v, &flag, p)) - w_complex_object(v, flag, p); + else if (!w_ref(v, p)) + w_complex_object(v, p); p->depth--; } static void -w_complex_object(PyObject *v, char flag, WFILE *p) +w_complex_object(PyObject *v, WFILE *p) { Py_ssize_t i, n; @@ -530,14 +700,14 @@ w_complex_object(PyObject *v, char flag, WFILE *p) int overflow; long x = PyLong_AsLongAndOverflow(v, &overflow); if (overflow) { - w_PyLong((PyLongObject *)v, flag, p); + w_PyLong((PyLongObject *)v, p); } else { #if SIZEOF_LONG > 4 long y = Py_ARITHMETIC_RIGHT_SHIFT(long, x, 31); if (y && y != -1) { /* Too large for TYPE_INT */ - w_PyLong((PyLongObject*)v, flag, p); + w_PyLong((PyLongObject*)v, p); } else #endif @@ -827,47 +997,46 @@ w_clear_refs(WFILE *wf) if (wf->hashtable != NULL) { _Py_hashtable_destroy(wf->hashtable); } + PyMem_Free(wf->refs); + PyMem_Free(wf->refpos); +} + +/* Write a buffer to a file. Set an exception on failure. */ +static void +w_fwrite(const char *buf, size_t n, FILE *fp) +{ + if (fwrite(buf, 1, n, fp) != n) { + int saved_errno = errno; + if (PyErr_CheckSignals()) { + /* The signal handler has raised an exception. */ + return; + } + errno = saved_errno; + PyErr_SetFromErrno(PyExc_OSError); + } } /* version currently has no effect for writing ints. */ void -PyMarshal_WriteLongToFile(long x, FILE *fp, int version) +PyMarshal_WriteLongToFile(long x, FILE *fp, int Py_UNUSED(version)) { char buf[4]; - WFILE wf; - memset(&wf, 0, sizeof(wf)); - wf.fp = fp; - wf.ptr = wf.buf = buf; - wf.end = wf.ptr + sizeof(buf); - wf.error = false; - wf.version = version; - w_long(x, &wf); - w_flush(&wf); - assert(!wf.error || PyErr_Occurred()); + w_put_uint32(buf, (uint32_t)x); + w_fwrite(buf, sizeof(buf), fp); } void PyMarshal_WriteObjectToFile(PyObject *x, FILE *fp, int version) { - char buf[BUFSIZ]; - WFILE wf; - if (PySys_Audit("marshal.dumps", "Oi", x, version) < 0) { - return; /* caller must check PyErr_Occurred() */ - } - memset(&wf, 0, sizeof(wf)); - wf.fp = fp; - wf.ptr = wf.buf = buf; - wf.end = wf.ptr + sizeof(buf); - wf.error = false; - wf.version = version; - wf.allow_code = 1; - if (w_init_refs(&wf, version)) { + /* Object references are resolved once the whole object is written + (see w_ref()), so the output cannot be written to the file piece by + piece. */ + PyObject *str = _PyMarshal_WriteObjectToString(x, version, 1); + if (str == NULL) { return; /* caller must check PyErr_Occurred() */ } - w_object(x, &wf); - w_clear_refs(&wf); - w_flush(&wf); - assert(!wf.error || PyErr_Occurred()); + w_fwrite(PyBytes_AS_STRING(str), (size_t)PyBytes_GET_SIZE(str), fp); + Py_DECREF(str); } typedef struct { @@ -2003,6 +2172,7 @@ _PyMarshal_WriteObjectToString(PyObject *x, int version, int allow_code) return NULL; } w_object(x, &wf); + w_resolve_refs(&wf); w_clear_refs(&wf); if (wf.str != NULL) { const char *base = PyBytes_AS_STRING(wf.str); From d69021ed3d5b4dad860e32d15c1e53ca8b54b3ac Mon Sep 17 00:00:00 2001 From: "Gregory P. Smith" Date: Tue, 13 Oct 2026 03:48:23 +0000 Subject: [PATCH 2/2] Merge the gh-156504 NEWS entry into the gh-129724 one --- .../2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst | 13 +++++++------ .../2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst | 3 --- 2 files changed, 7 insertions(+), 9 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst diff --git a/Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst b/Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst index 301b146bd4cbc34..90db9a32b8c5afa 100644 --- a/Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst +++ b/Misc/NEWS.d/next/Library/2026-10-11-06-00-00.gh-issue-129724.mRsh1x.rst @@ -1,9 +1,10 @@ The output of :func:`marshal.dumps` and :func:`marshal.dump`, and so the content of :file:`.pyc` files, no longer depends on the reference counts of the serialized objects: it is no longer changed by unrelated references to -them held elsewhere in the process. An object is now written in a form that -can be referenced later in the data if and only if it occurs more than once -in the serialized value; this also applies to interned strings, which were -always written in that form. The format is unchanged and the data can be -read by previous Python versions that support the format version, but it is -not byte-for-byte identical to the data written by them. +them held elsewhere in the process nor, in the :term:`free-threaded build`, +by whether they have been used by other threads. An object is now written +in a form that can be referenced later in the data if and only if it occurs +more than once in the serialized value; this also applies to interned +strings, which were always written in that form. The format is unchanged +and the data can be read by previous Python versions that support the format +version, but it is not byte-for-byte identical to the data written by them. diff --git a/Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst b/Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst deleted file mode 100644 index 272ecba61c676ff..000000000000000 --- a/Misc/NEWS.d/next/Library/2026-10-11-06-00-01.gh-issue-156504.Tf7kPz.rst +++ /dev/null @@ -1,3 +0,0 @@ -In the :term:`free-threaded build`, the output of :func:`marshal.dumps` and -:func:`marshal.dump`, and so the content of :file:`.pyc` files, no longer -depends on whether the serialized objects have been used by other threads.