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

[Todd Fiala]

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,
Piotr?

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

Thanks,
Todd


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:
>> LLDB_DISABLE_O_CLOEXEC and LLDB_DISABLE_DUPFD_CLOEXEC, currently not
>> 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>