[Lldb-commits] [PATCH] Fix descriptor leak in multi-target debugging

[Greg Clayton]

Looks good.

> On Feb 6, 2015, at 6:21 AM, Pavel Labath <labath at google.com> wrote:
> 
> Hi clayborg, vharron,
> 
> When debugging two targets concurrently, the pseude terminal master fd from the first one would
> leak into the second. This fixes the problem by setting O_CLOEXEC on the master fd. Test
> included.
> 
> http://reviews.llvm.org/D7466
> 
> Files:
>  source/Utility/PseudoTerminal.cpp
>  test/functionalities/avoids-fd-leak/TestFdLeak.py
> 
> Index: source/Utility/PseudoTerminal.cpp
> ===================================================================
> --- source/Utility/PseudoTerminal.cpp
> +++ source/Utility/PseudoTerminal.cpp
> @@ -239,7 +239,7 @@
>         error_str[0] = '\0';
> 
>     pid_t pid = LLDB_INVALID_PROCESS_ID;
> -    if (OpenFirstAvailableMaster (O_RDWR, error_str, error_len))
> +    if (OpenFirstAvailableMaster (O_RDWR | O_CLOEXEC, error_str, error_len))
>     {
>         // Successfully opened our master pseudo terminal
> 
> @@ -258,7 +258,8 @@
>             if (OpenSlave (O_RDWR, error_str, error_len))
>             {
>                 // Successfully opened slave
> -                // We are done with the master in the child process so lets close it
> +
> +                // Master FD should have O_CLOEXEC set, but let's close it just in case...
>                 CloseMasterFileDescriptor ();
> 
> #if defined(TIOCSCTTY)
> Index: test/functionalities/avoids-fd-leak/TestFdLeak.py
> ===================================================================
> --- test/functionalities/avoids-fd-leak/TestFdLeak.py
> +++ test/functionalities/avoids-fd-leak/TestFdLeak.py
> @@ -36,6 +36,29 @@
>         self.assertTrue(process.GetExitStatus() == 0,
>                 "Process returned non-zero status. Were incorrect file descriptors passed?")
> 
> +    @skipIfWindows # The check for descriptor leakage needs to be implemented differently here.
> +    def test_fd_leak_multitarget (self):
> +        self.buildDefault()
> +        exe = os.path.join (os.getcwd(), "a.out")
> +
> +        target = self.dbg.CreateTarget(exe)
> +
> +        listener = lldb.SBListener()
> +        error = lldb.SBError()
> +        process1 = target.Launch (listener, None, None, None, None, None,
> +                self.get_process_working_directory(), 0, True, # stop at entry
> +                error)
> +        self.assertTrue(process1, PROCESS_IS_VALID)
> +        self.assertTrue(process1.GetState() == lldb.eStateStopped, "Process should have been stopped.")
> +
> +        target2 = self.dbg.CreateTarget(exe)
> +        process2 = target2.LaunchSimple (None, None, self.get_process_working_directory())
> +        self.assertTrue(process2, PROCESS_IS_VALID)
> +
> +        self.assertTrue(process2.GetState() == lldb.eStateExited, "Process should have exited.")
> +        self.assertTrue(process2.GetExitStatus() == 0,
> +                "Process returned non-zero status. Were incorrect file descriptors passed?")
> +
> 
> if __name__ == '__main__':
>     import atexit
> 
> EMAIL PREFERENCES
>  http://reviews.llvm.org/settings/panel/emailpreferences/
> <D7466.19482.patch>