[lldb-dev] [PATCH] Avoid leaking file descriptors.
tfiala at google.com
Tue Mar 25 14:01:08 PDT 2014
btw on this one:
I disabled the tests in that file that were failing intermittently for me,
I haven't been seeing this one at all:
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:
>> 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
>> 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:
>> 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
>> 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.
>> @Greg, sorry that it took longer than I thought it will.
>> lldb-dev mailing list
>> lldb-dev at cs.uiuc.edu
> 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...
More information about the lldb-dev