This is an archive of the discontinued LLVM Phabricator instance.

Fix TestFdLeak on Linux.
ClosedPublic

Authored by chaoren on Apr 22 2015, 4:55 PM.

Details

Summary

LLGS leaks pipes (when launched by lldb), sockets (when launched by platform),
and/or log file to the inferior. This should prevent all possible leaks.

Diff Detail

Repository
rL LLVM

Event Timeline

chaoren updated this revision to Diff 24263.Apr 22 2015, 4:55 PM
chaoren retitled this revision from to Fix TestFdLeak on Linux..
chaoren updated this object.
chaoren edited the test plan for this revision. (Show Details)
chaoren added reviewers: vharron, clayborg.
chaoren added a subscriber: Unknown Object (MLST).
clayborg requested changes to this revision.Apr 23 2015, 10:40 AM
clayborg edited edge metadata.

My main concern is that we allow the ProcessLaunchInfo to specify rules for things other than file STDIN, STDOUT and STDERR. This fix would close anything but STDIN/OUT/ERR. Maybe this functionality just wasn't implemented on Linux in which case this won't hurt anything, but we should look into that. I am not sure how ProcessLaunchInfo makes its way down into the NativeProcessLinux::Launch() function, but you should check it out.

source/Plugins/Process/Linux/NativeProcessLinux.cpp
1799–1801 ↗(On Diff #24263)

Don't we allow file rules for other files that should be opened/duped/closed/etc via the posix_spawn style file rules? If so, this would close all of those... Not sure if this just isn't implemented for linux?

This revision now requires changes to proceed.Apr 23 2015, 10:40 AM
chaoren updated this revision to Diff 24316.Apr 23 2015, 11:16 AM
chaoren edited edge metadata.

Don't close fds with file actions.

clayborg accepted this revision.Apr 23 2015, 11:22 AM
clayborg edited edge metadata.

Much better.

This revision is now accepted and ready to land.Apr 23 2015, 11:22 AM
This revision was automatically updated to reflect the committed changes.