Re: [PATCH v6 14/59] perf python: Add wrapper for perf_data file abstraction

sashiko-bot
2026-04-25T18:19:27Z

Sashiko AI review found 2 potential issue(s):
- [Medium] The `fd` argument in `perf.data(fd=...)` is silently ignored and overwritten, leading to a file descriptor leak and reading from the wrong source.
- [Medium] Opening a FIFO via `perf.data()` without releasing the Python GIL causes the Python process to hang indefinitely.
--

commit f0f569c0460c34314c3be2d7c3dfa4916ea585ce
Author: Ian Rogers <irogers@google.com>

perf python: Add wrapper for perf_data file abstraction

This commit adds a python wrapper around the perf_data struct. This wrapper
is intended to support session management within python scripts.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c
[ ... ]
> +static int pyrf_data__init(struct pyrf_data *pdata, PyObject *args, PyObject *kwargs)
> +{
> + static char *kwlist[] = { "path", "fd", NULL };
> + char *path = NULL;
> + int fd = -1;
[ ... ]
> + if (!path)
> + path = "perf.data";
[ ... ]
> + pdata->data.mode = PERF_DATA_MODE_READ;
> + pdata->data.file.fd = fd;
> + if (perf_data__open(&pdata->data) < 0) {

Does perf_data__open() overwrite the user-provided fd?

If the path defaults to "perf.data", it appears perf_data__open() will open
that file from the filesystem and overwrite pdata->data.file.fd with the new
file descriptor.

Alternatively, if the path is explicitly set to "-" to trigger pipe mode,
the underlying check_pipe() function unconditionally assigns STDIN_FILENO
for read operations.

Could this sequence cause the provided fd to be leaked and completely ignored?

Also, can opening a FIFO here block the entire Python process?

perf_data__open() eventually executes a blocking open(path, O_RDONLY) system
call.

Since the Python Global Interpreter Lock (GIL) is not released before
this blocking I/O operation (for example, by using Py_BEGIN_ALLOW_THREADS),
will this freeze all other Python threads and prevent Python-level signal
handlers like Ctrl+C from executing if the FIFO is waiting for a writer?

> + PyErr_Format(PyExc_IOError, "Failed to open perf data: %s",
> + pdata->data.path ? pdata->data.path : "perf.data");
> + return -1;
> + }
> + return 0;
> +}

--
Sashiko AI review · https://sashiko.dev/#/patchset/20260425174858.3922152-1-irogers@google.com?part=14