O_CLOEXEC-like functionality for llvm::raw_fd_ostream

Hello all,

I am looking into using LLVM streams more extensively in LLDB (which
currently rolls it's own stream classes). One of the things that's
missing for me to be able to do that is the ability to open a file
with the O_CLOEXEC flag (to prevent us leaking file descriptors into
the debugged process).

So I tried adding a F_NonInheritable flag to the raw_fd_ostream
constructor, which would map to O_CLOEXEC on unix, and non-inheritable
handles on windows. However, I encountered a discrepancy in the
current behavior there.

The current behavior on unix is to make the file descriptors
inheritable (as that is the platform default). On windows, the current
behavior is to make the handles *non*-inheritable (again, because of
platform default). Obviously, if we add this flag, we will probably
need to settle on a default behavior which is consistent across all
platforms.

Instead of going with the unix choice of inherit-by-default, I propose
to go with the windows behavior where one has to explicitly opt into
file handle inheritance. I believe that is the safer way, as when one
thinks about creating a process, he usually knows which files/handles
it wants it to inherit (so he can specify it), whereas if he is not
creating a process, it does not matter whether it ends up being
inheritable (and he can avoid it leaking by accident!).

What do you think about this proposal? Does anyone know why this might
be a bad idea?

PS: Currently the test suite passes on windows&linux regardless of
which way I set the default to be.

cheers,
pavel

Pavel Labath via llvm-dev <llvm-dev@lists.llvm.org> writes:

Hello all,

I am looking into using LLVM streams more extensively in LLDB (which
currently rolls it's own stream classes). One of the things that's
missing for me to be able to do that is the ability to open a file
with the O_CLOEXEC flag (to prevent us leaking file descriptors into
the debugged process).

So I tried adding a F_NonInheritable flag to the raw_fd_ostream
constructor, which would map to O_CLOEXEC on unix, and non-inheritable
handles on windows. However, I encountered a discrepancy in the
current behavior there.

The current behavior on unix is to make the file descriptors
inheritable (as that is the platform default). On windows, the current
behavior is to make the handles *non*-inheritable (again, because of
platform default). Obviously, if we add this flag, we will probably
need to settle on a default behavior which is consistent across all
platforms.

Instead of going with the unix choice of inherit-by-default, I propose
to go with the windows behavior where one has to explicitly opt into
file handle inheritance. I believe that is the safer way, as when one
thinks about creating a process, he usually knows which files/handles
it wants it to inherit (so he can specify it), whereas if he is not
creating a process, it does not matter whether it ends up being
inheritable (and he can avoid it leaking by accident!).

What do you think about this proposal? Does anyone know why this might
be a bad idea?

I like the idea of non-inheritable by default.

Cheers,
Rafael

Making O_CLOEXEC the default sounds good.

Non-inheritable, no doubt.
The created process is getting loads of handles it does not know about. I had to workaround similar situation before, spawning an intermediate process just to get rid of inherited handles.

Thanks for the feedback, I'll get on it.