<div dir="ltr">Apologies, I clearly misunderstood. I'll add this again.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 12, 2018 at 2:52 AM, Pavel Labath via lldb-commits <span dir="ltr"><<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think you misunderstood the purpose the purpose of this test. The<br>
test was there to make sure that *lldb* does not leak file descriptors<br>
into the inferior (when I wrote it a couple of years ago, we were<br>
leaking about half a dozen of them). The python version check was<br>
added there just because we found there are some leaks coming from<br>
python itself that we could not control (so we skipped the test with<br>
those versions).<br>
<br>
With this in mind, I think the test is still very much useful. I think<br>
a better cleanup would be to rewrite this test to not depend on python<br>
so much, e.g. by driving the test from a command line lldb client a'la<br>
the tests in `lldb/lit/Expr` (unfortunately I can't think of a way to<br>
check that an FD is not leaked in a different way than with a<br>
full-scale integration test).<br>
<br>
On 9 February 2018 at 22:48, Vedant Kumar via lldb-commits<br>
<div class="HOEnZb"><div class="h5"><<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a>> wrote:<br>
> Nice!<br>
><br>
> vedant<br>
><br>
>> On Feb 9, 2018, at 8:06 AM, Davide Italiano via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: davide<br>
>> Date: Fri Feb 9 08:06:39 2018<br>
>> New Revision: 324743<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=324743&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=324743&view=rev</a><br>
>> Log:<br>
>> [Testsuite] Remove leak tests, it's not useful anymore.<br>
>><br>
>> This only worked on MacOS, which now ships a newer version of<br>
>> python without this bug. As such, we don't leak the fd, and<br>
>> this test is not needed anymore (as it also hardcoded the python<br>
>> version in the check).<br>
>><br>
>> Removed:<br>
>> lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/Makefile<br>
>> lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/TestFdLeak.py<br>
>> lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/main.c<br>
>><br>
>> Removed: lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/Makefile<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile?rev=324742&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lldb/trunk/packages/<wbr>Python/lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/Makefile?rev=324742&view=<wbr>auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/Makefile (original)<br>
>> +++ lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/Makefile (removed)<br>
>> @@ -1,5 +0,0 @@<br>
>> -LEVEL = ../../make<br>
>> -<br>
>> -C_SOURCES := main.c<br>
>> -<br>
>> -include $(LEVEL)/Makefile.rules<br>
>><br>
>> Removed: lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/TestFdLeak.py<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=324742&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lldb/trunk/packages/<wbr>Python/lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/TestFdLeak.py?rev=324742&<wbr>view=auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/TestFdLeak.py (original)<br>
>> +++ lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/TestFdLeak.py (removed)<br>
>> @@ -1,108 +0,0 @@<br>
>> -"""<br>
>> -Test whether a process started by lldb has no extra file descriptors open.<br>
>> -"""<br>
>> -<br>
>> -from __future__ import print_function<br>
>> -<br>
>> -<br>
>> -import os<br>
>> -import lldb<br>
>> -from lldbsuite.test import lldbutil<br>
>> -from lldbsuite.test.lldbtest import *<br>
>> -from lldbsuite.test.decorators import *<br>
>> -<br>
>> -<br>
>> -def python_leaky_fd_version(test):<br>
>> - import sys<br>
>> - # Python random module leaks file descriptors on some versions.<br>
>> - if sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10):<br>
>> - return "Python random module leaks file descriptors in this python version"<br>
>> - return None<br>
>> -<br>
>> -<br>
>> -class AvoidsFdLeakTestCase(TestBase)<wbr>:<br>
>> -<br>
>> - NO_DEBUG_INFO_TESTCASE = True<br>
>> -<br>
>> - mydir = TestBase.compute_mydir(__file_<wbr>_)<br>
>> -<br>
>> - @expectedFailure(python_leaky_<wbr>fd_version, "<a href="http://bugs.freebsd.org/197376" rel="noreferrer" target="_blank">bugs.freebsd.org/197376</a>")<br>
>> - @expectedFailureAll(<br>
>> - oslist=['freebsd'],<br>
>> - bugnumber="<a href="http://llvm.org/pr25624" rel="noreferrer" target="_blank">llvm.org/pr25624</a> still failing with Python 2.7.10")<br>
>> - # The check for descriptor leakage needs to be implemented differently<br>
>> - # here.<br>
>> - @skipIfWindows<br>
>> - @skipIfTargetAndroid() # Android have some other file descriptors open by the shell<br>
>> - @skipIfDarwinEmbedded # <rdar://problem/33888742> # debugserver on ios has an extra fd open on launch<br>
>> - def test_fd_leak_basic(self):<br>
>> - self.do_test([])<br>
>> -<br>
>> - @expectedFailure(python_leaky_<wbr>fd_version, "<a href="http://bugs.freebsd.org/197376" rel="noreferrer" target="_blank">bugs.freebsd.org/197376</a>")<br>
>> - @expectedFailureAll(<br>
>> - oslist=['freebsd'],<br>
>> - bugnumber="<a href="http://llvm.org/pr25624" rel="noreferrer" target="_blank">llvm.org/pr25624</a> still failing with Python 2.7.10")<br>
>> - # The check for descriptor leakage needs to be implemented differently<br>
>> - # here.<br>
>> - @skipIfWindows<br>
>> - @skipIfTargetAndroid() # Android have some other file descriptors open by the shell<br>
>> - @skipIfDarwinEmbedded # <rdar://problem/33888742> # debugserver on ios has an extra fd open on launch<br>
>> - def test_fd_leak_log(self):<br>
>> - self.do_test(["log enable -f '/dev/null' lldb commands"])<br>
>> -<br>
>> - def do_test(self, commands):<br>
>> - self.build()<br>
>> - exe = self.getBuildArtifact("a.out")<br>
>> -<br>
>> - for c in commands:<br>
>> - self.runCmd(c)<br>
>> -<br>
>> - target = self.dbg.CreateTarget(exe)<br>
>> -<br>
>> - process = target.LaunchSimple(<br>
>> - None, None, self.get_process_working_<wbr>directory())<br>
>> - self.assertTrue(process, PROCESS_IS_VALID)<br>
>> -<br>
>> - self.assertTrue(<br>
>> - process.GetState() == lldb.eStateExited,<br>
>> - "Process should have exited.")<br>
>> - self.assertTrue(<br>
>> - process.GetExitStatus() == 0,<br>
>> - "Process returned non-zero status. Were incorrect file descriptors passed?")<br>
>> -<br>
>> - @expectedFailure(python_leaky_<wbr>fd_version, "<a href="http://bugs.freebsd.org/197376" rel="noreferrer" target="_blank">bugs.freebsd.org/197376</a>")<br>
>> - @expectedFailureAll(<br>
>> - oslist=['freebsd'],<br>
>> - bugnumber="<a href="http://llvm.org/pr25624" rel="noreferrer" target="_blank">llvm.org/pr25624</a> still failing with Python 2.7.10")<br>
>> - # The check for descriptor leakage needs to be implemented differently<br>
>> - # here.<br>
>> - @skipIfWindows<br>
>> - @skipIfTargetAndroid() # Android have some other file descriptors open by the shell<br>
>> - @skipIfDarwinEmbedded # <rdar://problem/33888742> # debugserver on ios has an extra fd open on launch<br>
>> - def test_fd_leak_multitarget(self)<wbr>:<br>
>> - self.build()<br>
>> - exe = self.getBuildArtifact("a.out")<br>
>> -<br>
>> - target = self.dbg.CreateTarget(exe)<br>
>> - breakpoint = target.<wbr>BreakpointCreateBySourceRegex(<br>
>> - 'Set breakpoint here', lldb.SBFileSpec("main.c", False))<br>
>> - self.assertTrue(breakpoint, VALID_BREAKPOINT)<br>
>> -<br>
>> - process1 = target.LaunchSimple(<br>
>> - None, None, self.get_process_working_<wbr>directory())<br>
>> - self.assertTrue(process1, PROCESS_IS_VALID)<br>
>> - self.assertTrue(<br>
>> - process1.GetState() == lldb.eStateStopped,<br>
>> - "Process should have been stopped.")<br>
>> -<br>
>> - target2 = self.dbg.CreateTarget(exe)<br>
>> - process2 = target2.LaunchSimple(<br>
>> - None, None, self.get_process_working_<wbr>directory())<br>
>> - self.assertTrue(process2, PROCESS_IS_VALID)<br>
>> -<br>
>> - self.assertTrue(<br>
>> - process2.GetState() == lldb.eStateExited,<br>
>> - "Process should have exited.")<br>
>> - self.assertTrue(<br>
>> - process2.GetExitStatus() == 0,<br>
>> - "Process returned non-zero status. Were incorrect file descriptors passed?")<br>
>><br>
>> Removed: lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/main.c<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c?rev=324742&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/lldb/trunk/packages/<wbr>Python/lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/main.c?rev=324742&view=<wbr>auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/main.c (original)<br>
>> +++ lldb/trunk/packages/Python/<wbr>lldbsuite/test/<wbr>functionalities/avoids-fd-<wbr>leak/main.c (removed)<br>
>> @@ -1,28 +0,0 @@<br>
>> -#include <sys/types.h><br>
>> -#include <sys/stat.h><br>
>> -#include <unistd.h><br>
>> -#include <errno.h><br>
>> -#include <stdio.h><br>
>> -<br>
>> -int<br>
>> -main (int argc, char const **argv)<br>
>> -{<br>
>> - struct stat buf;<br>
>> - int i, rv = 0; // Set breakpoint here.<br>
>> -<br>
>> - // Make sure stdin/stdout/stderr exist.<br>
>> - for (i = 0; i <= 2; ++i) {<br>
>> - if (fstat(i, &buf) != 0)<br>
>> - return 1;<br>
>> - }<br>
>> -<br>
>> - // Make sure no other file descriptors are open.<br>
>> - for (i = 3; i <= 256; ++i) {<br>
>> - if (fstat(i, &buf) == 0 || errno != EBADF) {<br>
>> - fprintf(stderr, "File descriptor %d is open.\n", i);<br>
>> - rv = 2;<br>
>> - }<br>
>> - }<br>
>> -<br>
>> - return rv;<br>
>> -}<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> lldb-commits mailing list<br>
>> <a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/lldb-commits</a><br>
><br>
> ______________________________<wbr>_________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/lldb-commits</a><br>
______________________________<wbr>_________________<br>
lldb-commits mailing list<br>
<a href="mailto:lldb-commits@lists.llvm.org">lldb-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/lldb-commits</a><br>
</div></div></blockquote></div><br></div>