Test launcher leaks temp file used to capture subproc stdout/err |
||||
Issue descriptionThe launcher inherits each temporary |output_file| into all child processes such that the last of N children inherits the handles of all N procs. Since some children may still be outstanding when the launcher reads and deletes the file for one given child, it will be unable to delete it if any other later child is still running at the time of delete. The launcher should use base::Launch's |handles_to_inherit| so rather than |inherit_handles| so that each child only inherits its one output handle.
,
Feb 3 2017
,
Feb 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9ea51a5ffa60046859be210ca5a76ae85abf5a94 commit 9ea51a5ffa60046859be210ca5a76ae85abf5a94 Author: grt <grt@chromium.org> Date: Fri Feb 03 14:42:31 2017 Only allow the desired child proc to inherit its output file. Prior to this change, other children would end up with references to an earlier child's stdout handle, thereby preventing it from being deleted. This change alone makes things better, but doesn't entirely plug the hole: OpenFile (used by ReadFileToString) inadvertently opens its handles for inheritance due to implementation details of the Microsoft CRT. There exists an undocumented "N" mode flag for opening file streams that disables allowing the opened file to be inherited into child processes. Another CL will address this. BUG= 671990 R=phajdan.jr@chromium.org Review-Url: https://codereview.chromium.org/2670213004 Cr-Commit-Position: refs/heads/master@{#447996} [modify] https://crrev.com/9ea51a5ffa60046859be210ca5a76ae85abf5a94/base/test/launcher/test_launcher.cc
,
Feb 6 2017
FYI: I have https://codereview.chromium.org/2678813003/ out for review to fix the second part of this.
,
Feb 13 2017
This should be fixed now. |
||||
►
Sign in to add a comment |
||||
Comment 1 by phajdan.jr@chromium.org
, Feb 1 2017