[lldb-dev] [PATCH] Avoid leaking file descriptors.

Todd Fiala tfiala at google.com
Tue Mar 25 14:01:08 PDT 2014

btw on this one:
> TestStepNoDebug.py

I disabled the tests in that file that were failing intermittently for me,
early today.

I haven't been seeing this one at all:
> TestCommandRegex.py

Regarding the patch, I ran the full test suite twice without any errors on
Ubuntu 12.04 x86_64. The changes look good here.  I see quite a bit of code
for handling Windows properly - have you verified it works okay there,

Greg - could you give this a whirl on MacOSX and make sure it looks good to
you?  Also see Piotr's springboard notes above.


On Tue, Mar 25, 2014 at 1:43 PM, Todd Fiala <tfiala at google.com> wrote:

> Looking at this now.
> On Sat, Mar 22, 2014 at 8:43 AM, Piotr Rak <piotr.rak at gmail.com> wrote:
>> Hi,
>> I've noticed that we leak file descriptors, since we don't use O_CLOEXEC
>> when opening files.
>> I've changed lldb_utility::PseudoTerminal, debugserver version of it and
>> lldb_private::File to set it while opening by default.
>> This can be disabled by defining:
>> integrated with any filesystem.
>> It also should compile if O_CLOEXEC and F_DUPFD_CLOEXEC aren't defined,
>> and give compilation warning.
>> Also added new File open option:
>> eOpenOptionInheritOnExecPosixFD that makes it fallback to old behavior,
>> however in whole code base it was not required, and could be removed...
>> I went pretty carefully through fork()/exec() and posix_spawn() codepaths
>> to ensure that everything is OK, and it looked it is, but might have missed
>> something.
>> None of them required changes, from what I have noticed, since use dup2
>> or posix_spawnattr_add*.
>> If any of your out of tree code needs to be adjusted,
>> PseudoTerminal::Fork looks like good example how to pass fd without leaking.
>> I've tested linux amd64 and haven't noticed any regressions comparing to
>> trunk, but some tests fail in non-deterministic way, so could mask it...
>> Most notably:
>> TestStepNoDebug.py
>> TestCommandRegex.py
>> Managed to make them fail with and without the patch running in separate.
>> I've  possibly broke debugserver when WITH_SPRINGBOARD is defined for
>> launch code path.
>> I don't know SBSLaunchApplicationForDebugging semantics, and have no idea
>> how to fix it, beside disabling it for PseudoTerminal if this option is
>> defined.
>> Those changes are not strictly required for debugserver yet I have made
>> them for completeness.
>> Both raw diff and git patch are attached.
>> Please test attached, and report back if any problems are noticed.
>> Cheers,
>> /Piotr
>> PS.
>> @Greg, sorry that it took longer than I thought it will.
>> _______________________________________________
>> lldb-dev mailing list
>> lldb-dev at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> --
> Todd Fiala | Software Engineer |  tfiala at google.com |  650-943-3180

Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140325/373d2453/attachment.html>

More information about the lldb-dev mailing list