gh-151292: _remote_debugging: Do not corrupt the binary file when hitting OverflowError

[maurycy]

The #151292 issue focuses on total_samples:u32 but a big problem is that we're leaving the file corrupted on any OverflowError.

So, for example, someone ran recording for an hour on the production - hit some limit and boom, it's unreadable because the header wasn't finalized:

2026-06-11T02:03:58.920689000+0200 maurycy@gimel /Users/maurycy/src/github.com/maurycy/cpython (vmremap 0bdde7f?) % head -c 128 /tmp/overflow.bin | xxd
00000000: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000010: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000020: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000030: 0000 0000 0000 0000 0000 0000 0000 0000  ................
00000040: 28b5 2ffd 0058 5c1d 013a d500 1621 8025  (./..X\..:...!.%
00000050: 49d2 0133 3003 331a 3330 3d32 35a9 ccc0  I..30.3.30=25...
00000060: 3aa9 049c b5d6 b69d d296 3122 4488 0c6d  :.........1"D..m
00000070: 0148 0150 014f 0946 32bd b931 318c b6de  .H.P.O.F2..11...

The PR fixes this by catching the exception and stopping the sampler gracefully and letting the export finalize. As a result, the finalizer cannot raise the OverflowError either.

There are many overflow errors:

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 632 to 636 in f8514dc

	if (entry->pending_rle_count > UINT32_MAX - writer->total_samples) {
	PyErr_SetString(PyExc_OverflowError,
	"too many samples for binary format");
	return -1;
	}

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 681 to 685 in f8514dc

	if (writer->total_samples == UINT32_MAX) {
	PyErr_SetString(PyExc_OverflowError,
	"too many samples for binary format");
	return -1;
	}

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 377 to 381 in f8514dc

	if (writer->string_count >= UINT32_MAX) {
	PyErr_SetString(PyExc_OverflowError,
	"too many strings for binary format");
	return -1;
	}

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 396 to 400 in f8514dc

	if ((uintmax_t)str_len > UINT32_MAX) {
	PyErr_Format(PyExc_OverflowError,
	"string length %zd exceeds binary format maximum %u",
	str_len, UINT32_MAX);
	return -1;

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 444 to 448 in f8514dc

	if (writer->frame_count >= UINT32_MAX) {
	PyErr_SetString(PyExc_OverflowError,
	"too many frames for binary format");
	return -1;
	}

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 493 to 497 in f8514dc

	if (writer->thread_count >= UINT32_MAX) {
	PyErr_SetString(PyExc_OverflowError,
	"too many threads for binary format");
	return NULL;
	}

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 1054 to 1059 in f8514dc

	if (interp_id_long > UINT32_MAX) {
	PyErr_Format(PyExc_OverflowError,
	"interpreter_id %lu exceeds maximum value %lu",
	interp_id_long, (unsigned long)UINT32_MAX);
	return -1;
	}

cpython/Modules/_remote_debugging/binary_io_writer.c

Lines 55 to 63 in f8514dc

	if (old_cap > SIZE_MAX / 2) {
	PyErr_SetString(PyExc_OverflowError, "Array capacity overflow");
	return -1;
	}
	size_t new_cap = old_cap * 2;
	if (new_cap > SIZE_MAX / elem_size1 || new_cap > SIZE_MAX / elem_size2) {
	PyErr_SetString(PyExc_OverflowError, "Array allocation size overflow");
	return -1;

Given the limits (u32 or u64) isn't easy to write a test for it, so I just settled for the interpreter.

• Issue: _remote_debugging: binary format only total_samples:u32 #151292

[maurycy]

@pablogsal Truth be told, I'm not 100% sure what's the best layer for handling "finalizable" exceptions like OverflowError.

The promise of "finalizing the file" is in the BinaryWriter:

cpython/Modules/_remote_debugging/module.c

Lines 1857 to 1864 in 3428959

	/*[clinic input]
	_remote_debugging.BinaryWriter.__exit__
	exc_type: object = None
	exc_val: object = None
	exc_tb: object = None
	Exit context manager, finalizing the file.
	[clinic start generated code]*/

This is not the case:

cpython/Lib/profiling/sampling/binary_collector.py

Lines 115 to 120 in 3428959

	def __exit__(self, exc_type, exc_val, exc_tb):
	"""Context manager exit - finalize unless there was an error."""
	if exc_type is None:
	self._writer.finalize()
	else:
	self._writer.close()

cpython/Modules/_remote_debugging/module.c

Line 1874 in 3428959

/* Only finalize on normal exit (no exception) */

Perhaps it's something as simple as weakening exc_type == Py_None, instead of doing a round-trip since the module should contain all the knowledge (ie: which exceptions are finalizable for it.)

This issue presents itself there since we've got both the unwinder and the collector in the bloc :

cpython/Lib/profiling/sampling/sample.py

Line 180 in 3428959

except (RuntimeError, UnicodeDecodeError, MemoryError, OSError):

...and the meaning of MemoryError, RuntimeError or OSError in the unwinder is non-fatal, but for the binary collector it might mean non-finalizable exception.

Of course - the very proper way to approach this is to maintain the finalizable state is in the writer itself, instead of relying on the exceptions... but that's way too much for this PR.

[pablogsal]

I agree that the module should own this knowledge, but I don’t think weakening exc_type == Py_None is the right contract. What we really need to know is whether the writer can still produce a valid file. So the clean version is probably: the writer tracks “still finalizable” internally. Format-limit errors that are detected before changing writer state leave it finalizable; I/O/compression/partial-write failures mark it broken. Then __exit__ and the collector can both ask the writer state instead of reasoning from broad exception classes.

For this PR I’m fine with the collector-level stop, but I’d avoid broadening __exit__ based only on OverflowError.

[pablogsal]

Actually I think for this PR, I’d prefer we make the C writer expose the real state instead of catching broad OverflowError. Te idea would be to:

• add a finalizable flag/state to BinaryWriter
• sample-count overflow sets “finalizable limit hit” before returning the error
• write/compression/finalize failures mark the writer broken
• BinaryCollector catches only that finalizable limit error and stops
• __exit__ finalizes if the writer is still finalizable, otherwise closes

[maurycy]

There's an obvious question whether we should raise OverflowError in so many places. I will clean this up, also.

[pablogsal]

Some comments

[pablogsal]

total_samples is a u64 in the header (HDR_SIZE_SAMPLES is 8), so "=I" here only reads half of it. On big-endian (the s390x buildbots) this reads the high word and the assert will fail. Can we read 64 bytes and unpack with "=Q"? btw TestBinaryFormatValidation below already has HDR_OFF_SAMPLES = 28, maybe we can reuse it.

[maurycy]

@pablogsal Definitely. Thank you for catching this. Sorry for not updating the PR immediately after merging #153425.

Changed to =Q and moved HDR_OFF_SAMPLES, and other consts, to BinaryFormatTestBase in 1231afe

I ack there's still #152892 (comment) pending

[pablogsal]

LGTM!

[maurycy]

@pablogsal Do we want to backport? There's #158551 that's much smaller.

[pablogsal]

@pablogsal Do we want to backport? There's #158551 that's much smaller.

Yes we should backport

[miss-islington-app]

Thanks @maurycy for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.15.
🐍🍒⛏🤖

[bedevere-app]

GH-158830 is a backport of this pull request to the 3.15 branch.