Re: [PATCH v3 12/15] perf python: Fix MetricGroup return type in perf.pyi

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Memory leak of PyObject references 'key' and 'value' in pyrf__metrics_cb upon success.
--

--- Patch [12]: [PATCH v3 12/15] perf python: Fix MetricGroup return type in perf.pyi ---
commit f9b984697afa89938af9de90c9da302a78dffa89
Author: Ian Rogers <[email protected]>

perf python: Fix MetricGroup return type in perf.pyi

The metrics() function can return a dictionary where the value is either
a string or a list of strings, so the type signature in the stub file
should be Union[str, List[str]].

> --- a/tools/perf/python/perf.pyi
> +++ b/tools/perf/python/perf.pyi
[ ... ]
> -def metrics() -> List[Dict[str, str]]:
> +def metrics() -> List[Dict[str, Union[str, List[str]]]]:

[Severity: High]
This isn't a bug introduced by this patch, but does the underlying C
implementation of the metrics function leak the 'key' and 'value' objects on
success?

Looking at pyrf__metrics_cb() in tools/perf/util/python.c, it appears
PyDict_SetItem is used without decrementing the reference counts afterward:

	PyObject *key = dict ? PyUnicode_FromString("MetricGroup") : NULL;
	PyObject *value = key ? pyrf__metrics_groups(pm) : NULL;

	if (!value || PyDict_SetItem(dict, key, value) != 0) {
		Py_XDECREF(key);
		Py_XDECREF(value);
		Py_XDECREF(dict);
		return -ENOMEM;
	}

Since PyDict_SetItem doesn't steal references, wouldn't the caller need to
decrement the reference counts for both 'key' and 'value' after a successful
insertion? Since they are only decremented in the error path, does this
result in a memory leak every time a metric group is processed?

>      """Get a list of available metrics.
>  
>      Returns:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12