Re: [PATCH v1 12/13] perf timechart: Add a --live mode to the TUI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: missing close for live mode pipe fd in ttimechart
--

commit dd0096e018bbc0f7d639c04dde6e4a581907e0c1
Author: Alice Rogers <alice.mei.rogers@gmail.com>

perf timechart: Add a --live mode to the TUI

Add a --live option to perf timechart and ttimechart.py that runs 'perf
record' writing CLOCK_MONOTONIC scheduler and power events, or I/O
syscall events when -I/--io-only is given, to a pipe and displays them
in the TUI as they happen.

> diff --git a/tools/perf/python/ttimechart.py b/tools/perf/python/ttimechart.py
> index 34ecf84e68c1d..61176aca2cf6e 100755
> --- a/tools/perf/python/ttimechart.py
> +++ b/tools/perf/python/ttimechart.py
[ ... ]
> @@ -1727,52 +2110,329 @@ class TimechartApp(App):
[ ... ]
> +    def start(self) -> None:
[ ... ]
> +        self.data_fd = self.proc.stdout.fileno()
> +        self.grow_pipe(self.data_fd)
> +        if output:
> +            # Save the data as it is passed on to be processed.
> +            tee_r, tee_w = os.pipe()
> +            self.tee_thread = threading.Thread(target=self.tee,
> +                                               args=(self.data_fd, tee_w, output),
> +                                               daemon=True)
> +            self.tee_thread.start()
> +            self.data_fd = tee_r
> +            self.grow_pipe(self.data_fd)

[Severity: Medium]
Does this code leak the file descriptor for the read end of the pipe?

When running in --live mode with an output file (-o), tee_r from os.pipe()
is assigned to self.data_fd. While self.data_fd is later passed to the C
extension (perf.data(fd=...)) where it is duplicated:

tools/perf/util/python.c:pyrf_data__init() {
	...
	fd = dup(fd);
	if (fd < 0) {
		PyErr_SetFromErrno(PyExc_OSError);
		return -1;
	}
	...
}

the original file descriptor is never closed in the Python code. This causes
the original tee_r (and potentially proc.stdout.fileno()) to remain open for
the lifetime of the TUI process.

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