[Lldb-commits] [lldb] r324743 - [Testsuite] Remove leak tests, it's not useful anymore.

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 12 02:52:34 PST 2018


I think you misunderstood the purpose the purpose of this test. The
test was there to make sure that *lldb* does not leak file descriptors
into the inferior (when I wrote it a couple of years ago, we were
leaking about half a dozen of them). The python version check was
added there just because we found there are some leaks coming from
python itself that we could not control (so we skipped the test with
those versions).

With this in mind, I think the test is still very much useful. I think
a better cleanup would be to rewrite this test to not depend on python
so much, e.g. by driving the test from a command line lldb client a'la
the tests in `lldb/lit/Expr` (unfortunately I can't think of a way to
check that an FD is not leaked in a different way than with a
full-scale integration test).

On 9 February 2018 at 22:48, Vedant Kumar via lldb-commits
<lldb-commits at lists.llvm.org> wrote:
> Nice!
>
> vedant
>
>> On Feb 9, 2018, at 8:06 AM, Davide Italiano via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>>
>> Author: davide
>> Date: Fri Feb  9 08:06:39 2018
>> New Revision: 324743
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=324743&view=rev
>> Log:
>> [Testsuite] Remove leak tests, it's not useful anymore.
>>
>> This only worked on MacOS, which now ships a newer version of
>> python without this bug. As such, we don't leak the fd, and
>> this test is not needed anymore (as it also hardcoded the python
>> version in the check).
>>
>> Removed:
>>    lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile
>>    lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py
>>    lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c
>>
>> Removed: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile?rev=324742&view=auto
>> ==============================================================================
>> --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile (original)
>> +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/Makefile (removed)
>> @@ -1,5 +0,0 @@
>> -LEVEL = ../../make
>> -
>> -C_SOURCES := main.c
>> -
>> -include $(LEVEL)/Makefile.rules
>>
>> Removed: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py?rev=324742&view=auto
>> ==============================================================================
>> --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py (original)
>> +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/TestFdLeak.py (removed)
>> @@ -1,108 +0,0 @@
>> -"""
>> -Test whether a process started by lldb has no extra file descriptors open.
>> -"""
>> -
>> -from __future__ import print_function
>> -
>> -
>> -import os
>> -import lldb
>> -from lldbsuite.test import lldbutil
>> -from lldbsuite.test.lldbtest import *
>> -from lldbsuite.test.decorators import *
>> -
>> -
>> -def python_leaky_fd_version(test):
>> -    import sys
>> -    # Python random module leaks file descriptors on some versions.
>> -    if sys.version_info >= (2, 7, 8) and sys.version_info < (2, 7, 10):
>> -        return "Python random module leaks file descriptors in this python version"
>> -    return None
>> -
>> -
>> -class AvoidsFdLeakTestCase(TestBase):
>> -
>> -    NO_DEBUG_INFO_TESTCASE = True
>> -
>> -    mydir = TestBase.compute_mydir(__file__)
>> -
>> -    @expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376")
>> -    @expectedFailureAll(
>> -        oslist=['freebsd'],
>> -        bugnumber="llvm.org/pr25624 still failing with Python 2.7.10")
>> -    # The check for descriptor leakage needs to be implemented differently
>> -    # here.
>> -    @skipIfWindows
>> -    @skipIfTargetAndroid()  # Android have some other file descriptors open by the shell
>> -    @skipIfDarwinEmbedded # <rdar://problem/33888742>  # debugserver on ios has an extra fd open on launch
>> -    def test_fd_leak_basic(self):
>> -        self.do_test([])
>> -
>> -    @expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376")
>> -    @expectedFailureAll(
>> -        oslist=['freebsd'],
>> -        bugnumber="llvm.org/pr25624 still failing with Python 2.7.10")
>> -    # The check for descriptor leakage needs to be implemented differently
>> -    # here.
>> -    @skipIfWindows
>> -    @skipIfTargetAndroid()  # Android have some other file descriptors open by the shell
>> -    @skipIfDarwinEmbedded # <rdar://problem/33888742>  # debugserver on ios has an extra fd open on launch
>> -    def test_fd_leak_log(self):
>> -        self.do_test(["log enable -f '/dev/null' lldb commands"])
>> -
>> -    def do_test(self, commands):
>> -        self.build()
>> -        exe = self.getBuildArtifact("a.out")
>> -
>> -        for c in commands:
>> -            self.runCmd(c)
>> -
>> -        target = self.dbg.CreateTarget(exe)
>> -
>> -        process = target.LaunchSimple(
>> -            None, None, self.get_process_working_directory())
>> -        self.assertTrue(process, PROCESS_IS_VALID)
>> -
>> -        self.assertTrue(
>> -            process.GetState() == lldb.eStateExited,
>> -            "Process should have exited.")
>> -        self.assertTrue(
>> -            process.GetExitStatus() == 0,
>> -            "Process returned non-zero status. Were incorrect file descriptors passed?")
>> -
>> -    @expectedFailure(python_leaky_fd_version, "bugs.freebsd.org/197376")
>> -    @expectedFailureAll(
>> -        oslist=['freebsd'],
>> -        bugnumber="llvm.org/pr25624 still failing with Python 2.7.10")
>> -    # The check for descriptor leakage needs to be implemented differently
>> -    # here.
>> -    @skipIfWindows
>> -    @skipIfTargetAndroid()  # Android have some other file descriptors open by the shell
>> -    @skipIfDarwinEmbedded # <rdar://problem/33888742>  # debugserver on ios has an extra fd open on launch
>> -    def test_fd_leak_multitarget(self):
>> -        self.build()
>> -        exe = self.getBuildArtifact("a.out")
>> -
>> -        target = self.dbg.CreateTarget(exe)
>> -        breakpoint = target.BreakpointCreateBySourceRegex(
>> -            'Set breakpoint here', lldb.SBFileSpec("main.c", False))
>> -        self.assertTrue(breakpoint, VALID_BREAKPOINT)
>> -
>> -        process1 = target.LaunchSimple(
>> -            None, None, self.get_process_working_directory())
>> -        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?")
>>
>> Removed: lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c?rev=324742&view=auto
>> ==============================================================================
>> --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c (original)
>> +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/avoids-fd-leak/main.c (removed)
>> @@ -1,28 +0,0 @@
>> -#include <sys/types.h>
>> -#include <sys/stat.h>
>> -#include <unistd.h>
>> -#include <errno.h>
>> -#include <stdio.h>
>> -
>> -int
>> -main (int argc, char const **argv)
>> -{
>> -    struct stat buf;
>> -    int i, rv = 0; // Set breakpoint here.
>> -
>> -    // Make sure stdin/stdout/stderr exist.
>> -    for (i = 0; i <= 2; ++i) {
>> -        if (fstat(i, &buf) != 0)
>> -            return 1;
>> -    }
>> -
>> -    // Make sure no other file descriptors are open.
>> -    for (i = 3; i <= 256; ++i) {
>> -        if (fstat(i, &buf) == 0 || errno != EBADF) {
>> -            fprintf(stderr, "File descriptor %d is open.\n", i);
>> -            rv = 2;
>> -        }
>> -    }
>> -
>> -    return rv;
>> -}
>>
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


More information about the lldb-commits mailing list