Re: [PATCH v7 21/59] perf python: Expose brstack in sample event

sashiko-bot
2026-04-25T23:13:03Z

Sashiko AI review found 1 potential issue(s):
- [High] Unbreakable Python reference cycle causes a permanent memory leak for every event with a branch stack.
--

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

perf python: Expose brstack in sample event

This commit implements pyrf_branch_entry and pyrf_branch_stack to allow
iteration over branch stack entries in perf python scripts.

> diff --git a/tools/perf/util/python.c b/tools/perf/util/python.c
> index 17d0ee15336f..256129fef4f8 100644
> --- a/tools/perf/util/python.c
> +++ b/tools/perf/util/python.c

[ ... ]

> +struct pyrf_branch_stack {
> + PyObject_HEAD
> + struct pyrf_event *pevent;
> + struct branch_entry *entries;
> + u64 nr;
> + u64 pos;
> +};

[ ... ]

> +static PyObject *pyrf_branch_stack__next(struct pyrf_branch_stack *pstack)
> +{
> + struct pyrf_branch_entry *pentry;
> +
> + if (pstack->pos >= pstack->nr)
> + return NULL;
> +
> + pentry = PyObject_New(struct pyrf_branch_entry, &pyrf_branch_entry__type);
> + if (!pentry)
> + return NULL;
> +
> + pentry->from = pstack->entries[pstack->pos].from;
> + pentry->to = pstack->entries[pstack->pos].to;
> + pentry->flags = pstack->entries[pstack->pos].flags;
> +
> + pstack->pos++;
> + return (PyObject *)pentry;
> +}

[ ... ]

> @@ -1307,6 +1462,27 @@ static PyObject *pyrf_event__new(const union perf_event *event, struct evsel *ev
> addr_location__exit(&al);
> }
> }
> + if (sample->branch_stack) {
> + struct branch_stack *bs = sample->branch_stack;
> + struct branch_entry *entries = perf_sample__branch_entries(sample);
> + struct pyrf_branch_stack *pstack;
> +
> + pstack = PyObject_New(struct pyrf_branch_stack, &pyrf_branch_stack__type);
> + if (pstack) {
> + Py_INCREF(pevent);
> + pstack->pevent = pevent;

Does this create an unbreakable Python reference cycle that leaks memory?

The code assigns pstack to pevent->brstack below, and here it assigns pevent
to pstack->pevent while taking a reference via Py_INCREF. Because neither
pyrf_event nor pyrf_branch_stack implements Python's cyclic garbage
collection protocol, the reference counts will never drop to zero.

This means the event object, the pstack object, and the calloc'd
pstack->entries array will be leaked for every sample processed.

Unlike the callchain implementation which requires pevent for lazy resolution,
pyrf_branch_stack__next() iterates entirely over the pstack->entries array
that is eagerly populated via memcpy() below.

Since pstack->pevent appears to be unused during iteration, could we avoid
the memory leak by entirely removing the Py_INCREF(pevent) and
pstack->pevent back-reference?

> + pstack->pos = 0;
> + pstack->nr = bs->nr;
> + pstack->entries = calloc(bs->nr, sizeof(struct branch_entry));
> + if (pstack->entries) {
> + memcpy(pstack->entries, entries,
> + bs->nr * sizeof(struct branch_entry));
> + pevent->brstack = (PyObject *)pstack;
> + } else {
> + Py_DECREF(pstack);
> + }
> + }
> + }
> return (PyObject *)pevent;
> }

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