Re: [PATCH v7 13/59] perf python: Use evsel in sample in pyrf_event

sashiko-bot
2026-04-25T23:22:22Z

Sashiko AI review found 3 potential issue(s):
- [High] Python bindings permanently leak `evsel` references for all non-sample events.
- [High] The patch introduces an `evsel` reference leak in `evlist__deliver_deferred_callchain` and `session__flush_deferred_samples` by overwriting `sample.evsel` without dropping the original reference.
- [Medium] Reusing a `struct perf_sample` inside an event loop silently leaks `evsel` references because `perf_sample__init()` overwrites the struct.
--

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

perf python: Use evsel in sample in pyrf_event

This commit avoids a duplicated evsel by using the one in the sample. It
also adds evsel__get and evsel__put to manage the evsel reference in
perf_sample.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 3015b9b4b4da..9b16d832810f 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3235,7 +3235,7 @@ int evsel__parse_sample(struct evsel *evsel, union perf_event *event,
> union u64_swap u;
>
> perf_sample__init(data, /*all=*/true);
> - data->evsel = evsel;
> + data->evsel = evsel__get(evsel);

Does this leak the evsel reference when parsing deferred callchains?

In tools/perf/util/session.c, both evlist__deliver_deferred_callchain()
and session__flush_deferred_samples() call evlist__parse_sample() which
populates sample and acquires the reference. Then they overwrite the
pointer without dropping the original reference:

sample.evsel = evlist__id2evsel(evlist, sample.id);

Could reusing a struct perf_sample inside an event loop silently leak
evsel references now?

In tools/perf/tests/perf-record.c (test__PERF_RECORD), the sample
variable is declared outside the event processing loop and passed to
evlist__parse_sample() repeatedly. Since evsel__parse_sample() internally
calls perf_sample__init(data, true) which zeroes the struct, it seems the
evsel pointer from the previous iteration is silently overwritten.

> data->cpu = data->pid = data->tid = -1;

[ ... ]

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 22cc57914ad4..2ba3556c4f16 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c

[ ... ]

> @@ -1798,8 +1796,6 @@ static PyObject *pyrf_evlist__read_on_cpu(struct pyrf_evlist *pevlist,
> return Py_None;
> }
>
> - pevent->evsel = evsel__get(evsel);
> -
> perf_mmap__consume(&md->core);
>
> err = evsel__parse_sample(evsel, &pevent->event, &pevent->sample);

Can this leak evsel references for non-sample events in the Python
bindings?

pyrf_evlist__read_on_cpu() unconditionally parses all events using
evsel__parse_sample(), acquiring the reference. However,
pyrf_sample_event__delete() (which calls perf_sample__exit()) is only
assigned to pyrf_sample_event__type. Other event types like
pyrf_mmap_event__type rely on Python's default object deallocator, which
does not call perf_sample__exit().

It appears this was fixed later in commit 5565efd86942 (perf python:
Refactor and add accessors to sample event).

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