Skip to content

Commit 354ef33

Browse files
authored
gh-149474: use Py_fopen in Binary{Reader,Writer} for audit hook and path-like support (#149524)
1 parent 45c47d2 commit 354ef33

8 files changed

Lines changed: 83 additions & 89 deletions

File tree

Lib/test/audit-tests.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,16 @@ def rl(name):
208208
else:
209209
return None
210210

211+
try:
212+
import _remote_debugging
213+
except ImportError:
214+
_remote_debugging = None
215+
216+
def rd(name):
217+
if _remote_debugging:
218+
return getattr(_remote_debugging, name, None)
219+
return None
220+
211221
# Try a range of "open" functions.
212222
# All of them should fail
213223
with TestHook(raise_on_events={"open"}) as hook:
@@ -225,6 +235,8 @@ def rl(name):
225235
(rl("append_history_file"), 0, None),
226236
(rl("read_init_file"), testfn),
227237
(rl("read_init_file"), None),
238+
(rd("BinaryWriter"), testfn, 1000, 0),
239+
(rd("BinaryReader"), testfn),
228240
]:
229241
if not fn:
230242
continue
@@ -258,6 +270,8 @@ def rl(name):
258270
("~/.history", "a") if rl("append_history_file") else None,
259271
(testfn, "r") if readline else None,
260272
("<readline_init_file>", "r") if readline else None,
273+
(testfn, "wb") if rd("BinaryWriter") else None,
274+
(testfn, "rb") if rd("BinaryReader") else None,
261275
]
262276
if i is not None
263277
],

Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import json
44
import os
5+
import pathlib
56
import random
67
import struct
78
import tempfile
@@ -814,6 +815,35 @@ def test_invalid_file_path(self):
814815
with BinaryReader("/nonexistent/path/file.bin") as reader:
815816
reader.replay_samples(RawCollector())
816817

818+
def test_path_arguments_round_trip(self):
819+
"""Reader and writer accept str, bytes or os.PathLike."""
820+
with tempfile.NamedTemporaryFile(suffix=".bin", delete=False) as f:
821+
filename = f.name
822+
self.temp_files.append(filename)
823+
824+
for path_arg in (filename, os.fsencode(filename), pathlib.Path(filename)):
825+
with self.subTest(path_type=type(path_arg).__name__):
826+
writer = _remote_debugging.BinaryWriter(path_arg, 1000, 0)
827+
writer.finalize()
828+
reader = _remote_debugging.BinaryReader(path_arg)
829+
info = reader.get_info()
830+
reader.close()
831+
self.assertEqual(info["sample_count"], 0)
832+
833+
def test_rejects_non_pathlike(self):
834+
"""Reader and writer raise TypeError on non-path-like filenames."""
835+
with self.assertRaises(TypeError):
836+
_remote_debugging.BinaryWriter(123, 1000, 0)
837+
with self.assertRaises(TypeError):
838+
_remote_debugging.BinaryReader(123)
839+
840+
def test_invalid_path_error_preserves_pathlib(self):
841+
"""Missing path: OSError carries the original path object, not a string."""
842+
missing = pathlib.Path("/i/do/not/exist")
843+
with self.assertRaises(FileNotFoundError) as cm:
844+
_remote_debugging.BinaryReader(missing)
845+
self.assertEqual(os.fspath(cm.exception.filename), os.fspath(missing))
846+
817847
def test_writer_handles_empty_stack_first_sample(self):
818848
"""BinaryWriter.write_sample tolerates an empty stack on a fresh thread.
819849
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix the binary writer in :mod:`profiling.sampling` not firing the audit
2+
(:pep:`578`) when creating the output file. The writer and the reader now
3+
accept any path-like object. Patch by Maurycy Pawłowski-Wieroński.

Modules/_remote_debugging/binary_io.h

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,6 @@ typedef struct {
253253
/* Main binary writer structure */
254254
typedef struct {
255255
FILE *fp;
256-
char *filename;
257256

258257
/* Write buffer for batched I/O */
259258
uint8_t *write_buffer;
@@ -311,10 +310,7 @@ typedef struct {
311310

312311
/* Main binary reader structure */
313312
typedef struct {
314-
char *filename;
315-
316313
#if USE_MMAP
317-
int fd;
318314
uint8_t *mapped_data;
319315
size_t mapped_size;
320316
#else
@@ -522,7 +518,7 @@ grow_array_inplace(void **ptr_addr, size_t count, size_t *capacity, size_t elem_
522518
* Create a new binary writer.
523519
*
524520
* Arguments:
525-
* filename: Path to output file
521+
* path: Path to output file
526522
* sample_interval_us: Sampling interval in microseconds
527523
* compression_type: COMPRESSION_NONE or COMPRESSION_ZSTD
528524
* start_time_us: Start timestamp in microseconds (from time.monotonic() * 1e6)
@@ -531,7 +527,7 @@ grow_array_inplace(void **ptr_addr, size_t count, size_t *capacity, size_t elem_
531527
* New BinaryWriter* on success, NULL on failure (PyErr set)
532528
*/
533529
BinaryWriter *binary_writer_create(
534-
const char *filename,
530+
PyObject *path,
535531
uint64_t sample_interval_us,
536532
int compression_type,
537533
uint64_t start_time_us
@@ -583,12 +579,12 @@ void binary_writer_destroy(BinaryWriter *writer);
583579
* Open a binary file for reading.
584580
*
585581
* Arguments:
586-
* filename: Path to input file
582+
* path: Path to input file
587583
*
588584
* Returns:
589585
* New BinaryReader* on success, NULL on failure (PyErr set)
590586
*/
591-
BinaryReader *binary_reader_open(const char *filename);
587+
BinaryReader *binary_reader_open(PyObject *path);
592588

593589
/*
594590
* Replay samples from binary file through a collector.

Modules/_remote_debugging/binary_io_reader.c

Lines changed: 15 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -358,37 +358,26 @@ reader_parse_frame_table(BinaryReader *reader, const uint8_t *data, size_t file_
358358
}
359359

360360
BinaryReader *
361-
binary_reader_open(const char *filename)
361+
binary_reader_open(PyObject *path)
362362
{
363363
BinaryReader *reader = PyMem_Calloc(1, sizeof(BinaryReader));
364364
if (!reader) {
365365
PyErr_NoMemory();
366366
return NULL;
367367
}
368368

369-
#if USE_MMAP
370-
reader->fd = -1; /* Explicit initialization for cleanup safety */
371-
#endif
372-
373-
reader->filename = PyMem_Malloc(strlen(filename) + 1);
374-
if (!reader->filename) {
375-
PyMem_Free(reader);
376-
PyErr_NoMemory();
377-
return NULL;
378-
}
379-
strcpy(reader->filename, filename);
380-
381369
#if USE_MMAP
382370
/* Open with mmap on Unix */
383-
reader->fd = open(filename, O_RDONLY);
384-
if (reader->fd < 0) {
385-
PyErr_SetFromErrnoWithFilename(PyExc_IOError, filename);
371+
FILE *fp = Py_fopen(path, "rb");
372+
if (!fp) {
386373
goto error;
387374
}
375+
int fd = fileno(fp);
388376

389377
struct stat st;
390-
if (fstat(reader->fd, &st) < 0) {
378+
if (fstat(fd, &st) < 0) {
391379
PyErr_SetFromErrno(PyExc_IOError);
380+
Py_fclose(fp);
392381
goto error;
393382
}
394383
reader->mapped_size = st.st_size;
@@ -400,14 +389,15 @@ binary_reader_open(const char *filename)
400389
*/
401390
#ifdef __linux__
402391
reader->mapped_data = mmap(NULL, reader->mapped_size, PROT_READ,
403-
MAP_PRIVATE | MAP_POPULATE, reader->fd, 0);
392+
MAP_PRIVATE | MAP_POPULATE, fd, 0);
404393
#else
405394
reader->mapped_data = mmap(NULL, reader->mapped_size, PROT_READ,
406-
MAP_PRIVATE, reader->fd, 0);
395+
MAP_PRIVATE, fd, 0);
407396
#endif
408397
if (reader->mapped_data == MAP_FAILED) {
409398
reader->mapped_data = NULL;
410399
PyErr_SetFromErrno(PyExc_IOError);
400+
Py_fclose(fp);
411401
goto error;
412402
}
413403

@@ -428,19 +418,20 @@ binary_reader_open(const char *filename)
428418

429419
/* Add file descriptor-level hints for better kernel I/O scheduling */
430420
#if defined(__linux__) && defined(POSIX_FADV_SEQUENTIAL)
431-
(void)posix_fadvise(reader->fd, 0, 0, POSIX_FADV_SEQUENTIAL);
421+
(void)posix_fadvise(fd, 0, 0, POSIX_FADV_SEQUENTIAL);
432422
if (reader->mapped_size > (64 * 1024 * 1024)) {
433-
(void)posix_fadvise(reader->fd, 0, 0, POSIX_FADV_WILLNEED);
423+
(void)posix_fadvise(fd, 0, 0, POSIX_FADV_WILLNEED);
434424
}
435425
#endif
436426

427+
(void)Py_fclose(fp);
428+
437429
uint8_t *data = reader->mapped_data;
438430
size_t file_size = reader->mapped_size;
439431
#else
440432
/* Use stdio on Windows */
441-
reader->fp = fopen(filename, "rb");
433+
reader->fp = Py_fopen(path, "rb");
442434
if (!reader->fp) {
443-
PyErr_SetFromErrnoWithFilename(PyExc_IOError, filename);
444435
goto error;
445436
}
446437

@@ -1263,8 +1254,6 @@ binary_reader_close(BinaryReader *reader)
12631254
return;
12641255
}
12651256

1266-
PyMem_Free(reader->filename);
1267-
12681257
#if USE_MMAP
12691258
if (reader->mapped_data) {
12701259
munmap(reader->mapped_data, reader->mapped_size);
@@ -1274,13 +1263,9 @@ binary_reader_close(BinaryReader *reader)
12741263
/* Clear sample_data which may point into the now-unmapped region */
12751264
reader->sample_data = NULL;
12761265
reader->sample_data_size = 0;
1277-
if (reader->fd >= 0) {
1278-
close(reader->fd);
1279-
reader->fd = -1; /* Mark as closed */
1280-
}
12811266
#else
12821267
if (reader->fp) {
1283-
fclose(reader->fp);
1268+
Py_fclose(reader->fp);
12841269
reader->fp = NULL;
12851270
}
12861271
if (reader->file_data) {

Modules/_remote_debugging/binary_io_writer.c

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,7 @@ write_sample_with_encoding(BinaryWriter *writer, ThreadEntry *entry,
717717
}
718718

719719
BinaryWriter *
720-
binary_writer_create(const char *filename, uint64_t sample_interval_us, int compression_type,
720+
binary_writer_create(PyObject *path, uint64_t sample_interval_us, int compression_type,
721721
uint64_t start_time_us)
722722
{
723723
BinaryWriter *writer = PyMem_Calloc(1, sizeof(BinaryWriter));
@@ -726,14 +726,6 @@ binary_writer_create(const char *filename, uint64_t sample_interval_us, int comp
726726
return NULL;
727727
}
728728

729-
writer->filename = PyMem_Malloc(strlen(filename) + 1);
730-
if (!writer->filename) {
731-
PyMem_Free(writer);
732-
PyErr_NoMemory();
733-
return NULL;
734-
}
735-
strcpy(writer->filename, filename);
736-
737729
writer->start_time_us = start_time_us;
738730
writer->sample_interval_us = sample_interval_us;
739731
writer->compression_type = compression_type;
@@ -799,9 +791,8 @@ binary_writer_create(const char *filename, uint64_t sample_interval_us, int comp
799791
}
800792
}
801793

802-
writer->fp = fopen(filename, "wb");
794+
writer->fp = Py_fopen(path, "wb");
803795
if (!writer->fp) {
804-
PyErr_SetFromErrnoWithFilename(PyExc_IOError, filename);
805796
goto error;
806797
}
807798

@@ -1193,7 +1184,7 @@ binary_writer_finalize(BinaryWriter *writer)
11931184
return -1;
11941185
}
11951186

1196-
if (fclose(writer->fp) != 0) {
1187+
if (Py_fclose(writer->fp) != 0) {
11971188
writer->fp = NULL;
11981189
PyErr_SetFromErrno(PyExc_IOError);
11991190
return -1;
@@ -1211,10 +1202,9 @@ binary_writer_destroy(BinaryWriter *writer)
12111202
}
12121203

12131204
if (writer->fp) {
1214-
fclose(writer->fp);
1205+
Py_fclose(writer->fp);
12151206
}
12161207

1217-
PyMem_Free(writer->filename);
12181208
PyMem_Free(writer->write_buffer);
12191209

12201210
#ifdef HAVE_ZSTD

Modules/_remote_debugging/clinic/module.c.h

Lines changed: 7 additions & 31 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)