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

[Pavel Labath]

I have committed a modified test case that works on mac.

If anyone knows why target.Launch(stop_on_entry=true) behaves differently
on linux and mac I would be very interested to know.

regards,
pavel


On 9 February 2015 at 17:19, Pavel Labath <labath at google.com> wrote:

> Funny, I thought I had tested this... Will check asap.
>
> On 9 February 2015 at 16:31, Ilia K <ki.stfu at gmail.com> wrote:
>
>> Hello,
>>
>> This test doesn't work on OS X:
>>
>> ======================================================================
>>> FAIL: test_fd_leak_multitarget (TestFdLeak.AvoidsFdLeakTestCase)
>>> ----------------------------------------------------------------------
>>> Traceback (most recent call last):
>>>   File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/lldbtest.py", line 690, in wrapper
>>>     func(*args, **kwargs)
>>>   File "/Users/testuser/build/workspace/LLDB_master_release_OSX/llvm_master/tools/lldb/test/functionalities/avoids-fd-leak/TestFdLeak.py", line 52, in test_fd_leak_multitarget
>>>     self.assertTrue(process1.GetState() == lldb.eStateStopped, "Process should have been stopped.")
>>> AssertionError: False is not True : Process should have been stopped.
>>> Config=x86_64-clang
>>>
>>>
>> Thanks,
>> Ilia
>>
>> On Mon, Feb 9, 2015 at 2:37 PM, Pavel Labath <labath at google.com> wrote:
>>
>>> Author: labath
>>> Date: Mon Feb  9 05:37:56 2015
>>> New Revision: 228570
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=228570&view=rev
>>> Log:
>>> Fix descriptor leak in multi-target debugging
>>>
>>> Summary:
>>> 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.
>>>
>>> Reviewers: clayborg, vharron
>>>
>>> Subscribers: lldb-commits
>>>
>>> Differential Revision: http://reviews.llvm.org/D7466
>>>
>>> Modified:
>>>     lldb/trunk/source/Utility/PseudoTerminal.cpp
>>>     lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py
>>>
>>> Modified: lldb/trunk/source/Utility/PseudoTerminal.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/PseudoTerminal.cpp?rev=228570&r1=228569&r2=228570&view=diff
>>>
>>> ==============================================================================
>>> --- lldb/trunk/source/Utility/PseudoTerminal.cpp (original)
>>> +++ lldb/trunk/source/Utility/PseudoTerminal.cpp Mon Feb  9 05:37:56 2015
>>> @@ -239,7 +239,7 @@ PseudoTerminal::Fork (char *error_str, s
>>>          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 @@ PseudoTerminal::Fork (char *error_str, s
>>>              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)
>>>
>>> Modified: lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=228570&r1=228569&r2=228570&view=diff
>>>
>>> ==============================================================================
>>> --- lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py
>>> (original)
>>> +++ lldb/trunk/test/functionalities/avoids-fd-leak/TestFdLeak.py Mon
>>> Feb  9 05:37:56 2015
>>> @@ -36,6 +36,29 @@ class AvoidsFdLeakTestCase(TestBase):
>>>          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
>>>
>>>
>>> _______________________________________________
>>> lldb-commits mailing list
>>> lldb-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150209/6c3e1912/attachment.html>