tree 2d2f1954a5879134914a763bee0c2824acafb0d7
parent 853d0534797ffd8aa3959ce793c9099fdd9c510c
author Ricky Liang <jcliang@chromium.org> 1464078754 +0800
committer ChromeOS Commit Bot <chromeos-commit-bot@chromium.org> 1464440580 +0000

debugd: Close bound FDs when debugd is done with them

Commit 476d59a ("Fix Process::BindFd when passing the same fd
to the child.") in libbrillo changed to not close a FD in
the parent process after fork if the FD was bound. As a result
it causes FD leaks in several places in debugd.

For example, chrome://tracing is broken because the pipe FD is
not closed after debugd handles kSystraceStop DBus call. Another
exmaple is the "Store Logs" in chrome://net-internals/#chromeos
where the logs are stored but debugd never closes the output FD.

We should close the bound FDs in the parent process when debugd
is done with them. This patch overrides BindFd() in SandboxedProcess
to record all the bound FDs and close all the bound FDs in the
destructor. The only place that SandboxedProcess is not used is
in DebugLogsTool::GetDebugLogs(), where the FD is closed explicitly
after the subprocess finishes.

Also the process that runs `systrace.sh stop` does not need to
be ProcessWithOutput, so change it to SandboxedProcess.

BUG=chromium:613122
TEST=On Chrome OS: chrome://tracing
  start a trace, be sure to enable "System tracing"
  stop trace
  => Chrome successfully imports trace data
     (before this patch, Chrome would hang, waiting for debugd to respond)
TEST=On Chrome OS: chrome://net-internals/#chromeos
  click on "Store Debug Logs" and wait for logs to be created
  run `lsof -p $(pgrep debugd)`
  => no FD leak in debugd
     (before this patch, the debugd would leave the logs output file opened)

Change-Id: I78ca534dc654e4dbc79d37a5e0278e2eb18c0e4e
Signed-off-by: Ricky Liang <jcliang@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/346762
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
(cherry picked from commit 1ef73e543200fb7150d2bfb223f4a398067e8599)
Reviewed-on: https://chromium-review.googlesource.com/348120