I'm done with O_CLOEXEC

classic Classic list List threaded Threaded
29 messages Options
12
Reply | Threaded
Open this post in threaded view
|

I'm done with O_CLOEXEC

Ryan Lortie
karaj,

For those unfamiliar with the issue: when a process is created on UNIX
via naive fork() and exec(), the default is that the process will
inherit all of the file descriptors of the parent.  This makes a lot of
sense for stdin, stdout and stderr, but almost nothing else.

This has been the cause of a lot of strange problems over the years.
The typical example: a process will open a listener socket at some point
and sometime later will call a library function that does a naive
fork()/exec() of a helper process that hangs around past the lifetime of
the original program.  When you try to restart the first program, the
socket is still being held open by the helper and the new instance can't
bind it again.

There are two fixes to this problem.

The first one, which we have been pursuing during the past several
years, is to try to mark every file descriptor that we create as
O_CLOEXEC.  This is particularly fun in multi-threaded programs because
it means that we have a race between the creation of a file descriptor
and marking it O_CLOEXEC vs. a fork() that may be happening in another
thread.  This has led to the creation of a whole bunch of new syscalls
to allow creation of file descriptors that already have O_CLOEXEC set
from the start, thus avoiding the race.  We have tried to use these
syscalls where possible, but they usually are not part of POSIX.
Somethings they are completely unavailable, even in Linux, or when they
are available, they have other annoying limitations.

The other fix to the problem is one that we have had in place for a long
time in the g_spawn_*() family of APIs, and also in the new GSubprocess
API.  The trick involves close()ing all fds (except stdin/out/err) each
time we do a fork()/exec() pair.

Assuming it is practised universally, only one of these fixes is
necessary.

Today I am suggesting that we completely abandon our attempts to follow
the first approach.  I'm done with O_CLOEXEC.

What led me to this was the dup3() system call.  This is a variant of
dup2() that was added (as part of the efforts mentioned above) to avoid
the O_CLOEXEC race:

  int dup3(int oldfd, int newfd, int flags);

unfortunately:

  dup3(0, -1, 0)                          = -1 EBADF (Bad file
  descriptor)

which means that using this as a stand-in for dup() is a no-go.  I could
probably work around that by creating a new eventfd() or unbound UNIX
socket in order to get a new fd number (while being careful to mark it
as O_CLOEXEC as well) before using dup3().  We could probably also get
this fixed in Linux, but dup3() has already been widely copied and we
would then have to go about detecting which implementations are working
and which aren't, and include a fallback (which would have to be
implemented using the same dirty hacks mentioned above).  I've had
enough with these games, and this isn't really about dup3() anyway.

O_CLOEXEC is useless.

Okay.  O_CLOEXEC is useful for one thing: when spawning a new process
using fork()/exec(), you may want to know if exec() worked.  An old
trick for this is to create a pipe and mark the writer end O_CLOEXEC.
The reader end will read EOF (due to the close of the writer) once
exec() has succeeded.  Otherwise, you can indicate the error by sending
some other data through the pipe and calling exit().

Aside from that, O_CLOEXEC is useless.

So: starting today I'm going to stop worrying about O_CLOEXEC being set
on every file descriptor that GLib creates.  I'm not going to go and
retroactively tear things out where they are already working, unless it
would provide a substantial cleanup or fixes an actual bug.  I'm not
just going to go around looking for #ifdefs to remove.

I believe this is justified for a few reasons:

 - during the GSubprocess discussion, I originally held the opposite
 opinion, but eventually became convinced (by Colin) to see the
 inherit-by-default behaviour of exec() as nothing more than a
 questionable implementation detail of the underlying OS.  Consequently,
 at the high level, GSubprocess provides an API that gives the caller
 direct control over what is inherited and what is not, and that's just
 the way that it should be.

 - this behaviour is not limited to GSubprocess.  Closing all fds before
 calling exec() is a common practice in modern libraries and runtimes,
 and for good reason.

 - fixing the few places that we spawn other programs is massively
 preferable to fixing the hundreds or thousands of places that we create
 new file descriptors

 - in the world of D-Bus activation, direct spawning of long-lived
 helper processes is just not something that we do anymore anyway.  fds
 are not the only thing we have to worry about here.  How about which
 cgroup the helper ends up in?

 - it's 2015 and I'm wondering why system() hasn't been deprecated
 already


Unfortunately, the drawback: it will no longer be safe, in general, to
use GLib with broken old libraries that don't close() fds before exec().

I'm sure others will have an opinion about this.  Did I miss something
important in my argument?

Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Christian Hergert
On 03/20/2015 01:43 PM, Ryan Lortie wrote:

> karaj,
>
> For those unfamiliar with the issue: when a process is created on UNIX
> via naive fork() and exec(), the default is that the process will
> inherit all of the file descriptors of the parent.  This makes a lot of
> sense for stdin, stdout and stderr, but almost nothing else.
>
> This has been the cause of a lot of strange problems over the years.
> The typical example: a process will open a listener socket at some point
> and sometime later will call a library function that does a naive
> fork()/exec() of a helper process that hangs around past the lifetime of
> the original program.  When you try to restart the first program, the
> socket is still being held open by the helper and the new instance can't
> bind it again.
>
> There are two fixes to this problem.

[..snip..]

This makes me happy. I don't think I've actually seen any of this stuff
handled right. Not to mention that close() itself is basically broken in
multi-threaded scenarios on Linux (Linus says to basically "just not
worry about it" if you get EINTR, which may or may not have succeeded,
and then the FD entry taken by another thread).

What I would welcome, is a function that says "glib, close all FDs you
know about or that you created". If all the libraries did that, at least
it would be possible for applications to maybe, sorta, do the right
thing. (That would push the synchronization responsibility during
fork()/exec() to the application).

-- Christian
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Paul Davis


On Fri, Mar 20, 2015 at 7:29 PM, Christian Hergert <[hidden email]> wrote:



This makes me happy. I don't think I've actually seen any of this stuff handled right. Not to mention that close() itself is basically broken in multi-threaded scenarios on Linux (Linus says to basically "just not worry about it" if you get EINTR, which may or may not have succeeded, and then the FD entry taken by another thread).

What I would welcome, is a function that says "glib, close all FDs you know about or that you created". If all the libraries did that, at least it would be possible for applications to maybe, sorta, do the right thing. (That would push the synchronization responsibility during fork()/exec() to the application).

no, it wouldn't.

as a pango user, do i call pango_close_all_fds_before_exec() or does gtk? or gdk? or ...

as a libfftw3 user, do i call fftw2_close_all_fds_before_exec() or does some other library that also uses it? (which i may know that i am using, or i may not (via loading some arbitrary module).

"call the close_before_exec() for all libraries that i know i explicitly call into and pray that the rest do the right things for other libraries that i don't explicitly use" .... this is a much weaker proposition.

what you want is "close_all_fds_before_exec()" that just gets the job done, in one place, in the application.
 

_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Matthias Clasen-2
In reply to this post by Ryan Lortie
So,  you found that dup3 doesn't do what you want, and now you want to
throw out the baby with the bathwater and just say "I don't care
anymore if we leak fds" ?

On Fri, Mar 20, 2015 at 4:43 PM, Ryan Lortie <[hidden email]> wrote:

> karaj,
>
> For those unfamiliar with the issue: when a process is created on UNIX
> via naive fork() and exec(), the default is that the process will
> inherit all of the file descriptors of the parent.  This makes a lot of
> sense for stdin, stdout and stderr, but almost nothing else.
>
> This has been the cause of a lot of strange problems over the years.
> The typical example: a process will open a listener socket at some point
> and sometime later will call a library function that does a naive
> fork()/exec() of a helper process that hangs around past the lifetime of
> the original program.  When you try to restart the first program, the
> socket is still being held open by the helper and the new instance can't
> bind it again.
>
> There are two fixes to this problem.
>
> The first one, which we have been pursuing during the past several
> years, is to try to mark every file descriptor that we create as
> O_CLOEXEC.  This is particularly fun in multi-threaded programs because
> it means that we have a race between the creation of a file descriptor
> and marking it O_CLOEXEC vs. a fork() that may be happening in another
> thread.  This has led to the creation of a whole bunch of new syscalls
> to allow creation of file descriptors that already have O_CLOEXEC set
> from the start, thus avoiding the race.  We have tried to use these
> syscalls where possible, but they usually are not part of POSIX.
> Somethings they are completely unavailable, even in Linux, or when they
> are available, they have other annoying limitations.
>
> The other fix to the problem is one that we have had in place for a long
> time in the g_spawn_*() family of APIs, and also in the new GSubprocess
> API.  The trick involves close()ing all fds (except stdin/out/err) each
> time we do a fork()/exec() pair.
>
> Assuming it is practised universally, only one of these fixes is
> necessary.
>
> Today I am suggesting that we completely abandon our attempts to follow
> the first approach.  I'm done with O_CLOEXEC.
>
> What led me to this was the dup3() system call.  This is a variant of
> dup2() that was added (as part of the efforts mentioned above) to avoid
> the O_CLOEXEC race:
>
>   int dup3(int oldfd, int newfd, int flags);
>
> unfortunately:
>
>   dup3(0, -1, 0)                          = -1 EBADF (Bad file
>   descriptor)
>
> which means that using this as a stand-in for dup() is a no-go.  I could
> probably work around that by creating a new eventfd() or unbound UNIX
> socket in order to get a new fd number (while being careful to mark it
> as O_CLOEXEC as well) before using dup3().  We could probably also get
> this fixed in Linux, but dup3() has already been widely copied and we
> would then have to go about detecting which implementations are working
> and which aren't, and include a fallback (which would have to be
> implemented using the same dirty hacks mentioned above).  I've had
> enough with these games, and this isn't really about dup3() anyway.
>
> O_CLOEXEC is useless.
>
> Okay.  O_CLOEXEC is useful for one thing: when spawning a new process
> using fork()/exec(), you may want to know if exec() worked.  An old
> trick for this is to create a pipe and mark the writer end O_CLOEXEC.
> The reader end will read EOF (due to the close of the writer) once
> exec() has succeeded.  Otherwise, you can indicate the error by sending
> some other data through the pipe and calling exit().
>
> Aside from that, O_CLOEXEC is useless.
>
> So: starting today I'm going to stop worrying about O_CLOEXEC being set
> on every file descriptor that GLib creates.  I'm not going to go and
> retroactively tear things out where they are already working, unless it
> would provide a substantial cleanup or fixes an actual bug.  I'm not
> just going to go around looking for #ifdefs to remove.
>
> I believe this is justified for a few reasons:
>
>  - during the GSubprocess discussion, I originally held the opposite
>  opinion, but eventually became convinced (by Colin) to see the
>  inherit-by-default behaviour of exec() as nothing more than a
>  questionable implementation detail of the underlying OS.  Consequently,
>  at the high level, GSubprocess provides an API that gives the caller
>  direct control over what is inherited and what is not, and that's just
>  the way that it should be.
>
>  - this behaviour is not limited to GSubprocess.  Closing all fds before
>  calling exec() is a common practice in modern libraries and runtimes,
>  and for good reason.
>
>  - fixing the few places that we spawn other programs is massively
>  preferable to fixing the hundreds or thousands of places that we create
>  new file descriptors
>
>  - in the world of D-Bus activation, direct spawning of long-lived
>  helper processes is just not something that we do anymore anyway.  fds
>  are not the only thing we have to worry about here.  How about which
>  cgroup the helper ends up in?
>
>  - it's 2015 and I'm wondering why system() hasn't been deprecated
>  already
>
>
> Unfortunately, the drawback: it will no longer be safe, in general, to
> use GLib with broken old libraries that don't close() fds before exec().
>
> I'm sure others will have an opinion about this.  Did I miss something
> important in my argument?
>
> Cheers
> _______________________________________________
> gtk-devel-list mailing list
> [hidden email]
> https://mail.gnome.org/mailman/listinfo/gtk-devel-list
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Ryan Lortie
On Fri, Mar 20, 2015, at 23:33, Matthias Clasen wrote:
> So,  you found that dup3 doesn't do what you want, and now you want to
> throw out the baby with the bathwater and just say "I don't care
> anymore if we leak fds" ?

dup3() was a bit of a "straw that broke the camel's back" case.  I could
point at the existence of g_unix_open_pipe() as a similarly ridiculous
case, or many others.

I'm also not impressed by the inaccurate categorisation.  I thought I
explained fairly clearly why I believe that leaked fds will _not_ be the
case, even without O_CLOEXEC.

I was looking for some slightly more constructive arguments...

Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Jasper St. Pierre
Sorry, I'm not overly familiar with this sort of stuff.

Right now, we use raw fork/exec in mutter where we need to do some tricky management and explicitly leak an FD into the correct place [0]. Does this mean that from now on, glib might leak an FD and we need to be prepared to handle that? Refactoring the code to use a child setup func and using g_spawn isn't quite really what I want to do (can I even leak an FD made with socketpair through in that case?), but I want to be aware of what might break in the future, and whose bug it should be.

I know it's difficult to set a policy about this, but is there anything I can do to prevent too much damage in the future? If I file a patch against glib for where it might not set CLOEXEC with an easy flag the syscall, will you accept it, or are you going to reject it to stop me from relying on CLOEXEC?

On Fri, Mar 20, 2015 at 10:10 PM, Ryan Lortie <[hidden email]> wrote:
On Fri, Mar 20, 2015, at 23:33, Matthias Clasen wrote:
> So,  you found that dup3 doesn't do what you want, and now you want to
> throw out the baby with the bathwater and just say "I don't care
> anymore if we leak fds" ?

dup3() was a bit of a "straw that broke the camel's back" case.  I could
point at the existence of g_unix_open_pipe() as a similarly ridiculous
case, or many others.

I'm also not impressed by the inaccurate categorisation.  I thought I
explained fairly clearly why I believe that leaked fds will _not_ be the
case, even without O_CLOEXEC.

I was looking for some slightly more constructive arguments...

Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list



--
  Jasper

_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Ryan Lortie
In reply to this post by Christian Hergert
On Fri, Mar 20, 2015, at 20:29, Christian Hergert wrote:
> What I would welcome, is a function that says "glib, close all FDs you
> know about or that you created". If all the libraries did that, at least
> it would be possible for applications to maybe, sorta, do the right
> thing. (That would push the synchronization responsibility during
> fork()/exec() to the application).

I don't think this is the correct approach.

The application should not have to be aware of what GLib is doing, or
even that it is using GLib at all (if GLib is pulled in by some other
intermediate library).  Much less for any other libraries that it may be
using.

What the application should be aware of is simple: what does it want to
pass to a process that it is spawning?  Anything that it doesn't want to
pass should be closed (after the fork, before the exec).

There are many ways of accomplishing this.  Some systems have an
fdwalk() [read: "foreach open fd"] call designed to help you do this.
On Linux the common idiom is to iterate /proc/self/fd/, closing as you
go.  On other systems which lack either of these, it's still possible to
obtain the maximum possible file descriptor number and simply use a for
loop to close them all (even if you call close() on some fds that are
not really open).

[[ As an aside: it would be great if we had a variant of execve() that
took an array of fds.  The new process image would end up with the fds
in that array remapped as fd 0, fd 1, fd 2 (and so on) according to
their array position.  The truth is, however, with the "walk and close
all fds" tricks that are already widely known and practised, this would
only be a convenience. ]]

Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Ryan Lortie
In reply to this post by Jasper St. Pierre
hi,

On Sat, Mar 21, 2015, at 01:19, Jasper St. Pierre wrote:
> Right now, we use raw fork/exec in mutter where we need to do some tricky
> management and explicitly leak an FD into the correct place [0]. Does
> this
> mean that from now on, glib might leak an FD and we need to be prepared
> to
> handle that? Refactoring the code to use a child setup func and using
> g_spawn isn't quite really what I want to do (can I even leak an FD made
> with socketpair through in that case?), but I want to be aware of what
> might break in the future, and whose bug it should be.

I recommend using GSubprocess.

g_subprocess_launcher_take_fd() lets you give an fd (along with a
target_fd number).  This fd will appear in the newly-spawned process as
the "target" number you gave.  This is what I mean by code that spawns
processes having explicit control over what they do.

For example:

  int sv[2];

  socketpair (..., sv);
  g_subprocess_launcher_take_fd (launcher, sv[1], 3);
  g_subprocess_launcher_spawn (launcher, NULL, "/usr/bin/whatever");

will put the sv[1] end of the socket pair into the launched process as
fd 3.

> I know it's difficult to set a policy about this, but is there anything I
> can do to prevent too much damage in the future? If I file a patch
> against
> glib for where it might not set CLOEXEC with an easy flag the syscall,
> will
> you accept it, or are you going to reject it to stop me from relying on
> CLOEXEC?

I'm not sure.  It probably depends on the outcome of this thread.  I'm
leaning towards "we won't do it if it complicates the code".

Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Jürg Billeter
In reply to this post by Ryan Lortie
On Fri, 2015-03-20 at 16:43 -0400, Ryan Lortie wrote:

> What led me to this was the dup3() system call.  This is a variant of
> dup2() that was added (as part of the efforts mentioned above) to avoid
> the O_CLOEXEC race:
>
>   int dup3(int oldfd, int newfd, int flags);
>
> unfortunately:
>
>   dup3(0, -1, 0)                          = -1 EBADF (Bad file
>   descriptor)
>
> which means that using this as a stand-in for dup() is a no-go.

Doesn't the following standard POSIX functionality provide what you
want?

        fcntl(fd, F_DUPFD_CLOEXEC, 0)

Regards,
Jürg

_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Ryan Lortie
hi,

On Sat, Mar 21, 2015, at 01:27, Jürg Billeter wrote:
> Doesn't the following standard POSIX functionality provide what you
> want?
>
> fcntl(fd, F_DUPFD_CLOEXEC, 0)

Yes.  It does.  Thank you very much.

It seems that this is a (slightly) recent addition.  It's documented:

       F_DUPFD_CLOEXEC (int; since Linux 2.6.24)

so I'm sure we'll probably still need to write an ifdef with a
fallback...

The wider question about the usefulness of O_CLOEXEC still stands.

Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Jürg Billeter
On Sat, 2015-03-21 at 01:32 -0400, Ryan Lortie wrote:
> It seems that this is a (slightly) recent addition.  It's documented:
>
>        F_DUPFD_CLOEXEC (int; since Linux 2.6.24)
>
> so I'm sure we'll probably still need to write an ifdef with a
> fallback...

If you're referring to older Linux versions, I disagree. Even glibc has
dropped support for Linux < 2.6.32, ifdef may be needed for other
kernels, though, not sure.

> The wider question about the usefulness of O_CLOEXEC still stands.

I would keep using O_CLOEXEC as it's as close as we can get to the
behavior that should have been the default: don't implicitly inherit
file descriptors on exec.

Maybe there are applications out there that rely on correct file
descriptor flags and directly call fork/exec. You could try to convince
them to switch to GSubprocess (or work around the issue in their own
fork/exec code). However, as I think we all agree that O_CLOEXEC is the
best default behavior, I don't see why we should break these
applications.

Regards,
Jürg

_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Chris Vine
On Sat, 21 Mar 2015 06:59:41 +0100
Jürg Billeter <[hidden email]> wrote:

> On Sat, 2015-03-21 at 01:32 -0400, Ryan Lortie wrote:
> > It seems that this is a (slightly) recent addition.  It's
> > documented:
> >
> >        F_DUPFD_CLOEXEC (int; since Linux 2.6.24)
> >
> > so I'm sure we'll probably still need to write an ifdef with a
> > fallback...
>
> If you're referring to older Linux versions, I disagree. Even glibc
> has dropped support for Linux < 2.6.32, ifdef may be needed for other
> kernels, though, not sure.
>
> > The wider question about the usefulness of O_CLOEXEC still stands.
>
> I would keep using O_CLOEXEC as it's as close as we can get to the
> behavior that should have been the default: don't implicitly inherit
> file descriptors on exec.
>
> Maybe there are applications out there that rely on correct file
> descriptor flags and directly call fork/exec. You could try to
> convince them to switch to GSubprocess (or work around the issue in
> their own fork/exec code). However, as I think we all agree that
> O_CLOEXEC is the best default behavior, I don't see why we should
> break these applications.

Further, there are cases where porting to GSubprocess does not actually
do the job easily because (as I understand it) GSubprocessFlags
can be set to either close all descriptors on exec other than those for
stdin, stdout and stderr, or not close any descriptors on exec.

I have code which uses glib and also launches a helper child process
using fork/exec so that the helper process can run a VM with garbage
collection, for a particular purpose not relevant here.  The parent and
helper processes communicate via two pipes, which for various reasons
do not dup stdin and stdout on the helper process but act as
independent channels.  This means that the helper process's descriptors
for the pipes must be kept open on exec.  At present the code relies on
glib doing the right thing and looking after its own descriptors on
exec.  It would be a nuisance if that could not be relied on in the
future, which I think is your proposal.  (I understand your point that
that is not an assumption that users should be entitled to make, but it
IS the present behaviour and changing it in a stable series may
introduce breakage.)

However, if that is the proposal and you are going to go ahead with
it, can you provide a "close_all_except" convenience function to go
along with it which will walk open descriptors and close them but allow
the user to supply a list of user's descriptors which are not to be
closed, which the user can call between the fork and the exec, if that
can be done without calling async-signal-safe functions
(however, possibly walking the open descriptor list cannot be done
using only async-signal-safe functions, which would make it a
non-runner for multi-threaded programs).  Obviously the user could do
that herself, but at some inconvenience for code intended to run on a
number of platforms, some of which may have fdwalk() and some of which
may not, and for some of which fdwalk() may be async-signal-safe and
some of which may not.

Chris
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Bastien Nocera
In reply to this post by Ryan Lortie
On Sat, 2015-03-21 at 01:32 -0400, Ryan Lortie wrote:

> hi,
>
> On Sat, Mar 21, 2015, at 01:27, Jürg Billeter wrote:
> > Doesn't the following standard POSIX functionality provide what you
> > want?
> >
> >     fcntl(fd, F_DUPFD_CLOEXEC, 0)
>
> Yes.  It does.  Thank you very much.
>
> It seems that this is a (slightly) recent addition.  It's documented:
>
>        F_DUPFD_CLOEXEC (int; since Linux 2.6.24)
>
> so I'm sure we'll probably still need to write an ifdef with a
> fallback...

7 years old:
http://kernelnewbies.org/Linux_2_6_24

> The wider question about the usefulness of O_CLOEXEC still stands.

_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Florian Müllner-2
In reply to this post by Chris Vine
On Sat, Mar 21, 2015 at 12:31 PM Chris Vine <[hidden email]> wrote:

Further, there are cases where porting to GSubprocess does not actually
do the job easily because (as I understand it) GSubprocessFlags
can be set to either close all descriptors on exec other than those for
stdin, stdout and stderr, or not close any descriptors on exec.

Isn't that case covered by g_subprocess_launcher_take_fd()?

_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Matthias Clasen-2
In reply to this post by Ryan Lortie
On Fri, Mar 20, 2015 at 4:43 PM, Ryan Lortie <[hidden email]> wrote:

> The first one, which we have been pursuing during the past several
> years, is to try to mark every file descriptor that we create as
> O_CLOEXEC.  This is particularly fun in multi-threaded programs because
> it means that we have a race between the creation of a file descriptor
> and marking it O_CLOEXEC vs. a fork() that may be happening in another
> thread.  This has led to the creation of a whole bunch of new syscalls
> to allow creation of file descriptors that already have O_CLOEXEC set
> from the start, thus avoiding the race.  We have tried to use these
> syscalls where possible, but they usually are not part of POSIX.
> Somethings they are completely unavailable, even in Linux, or when they
> are available, they have other annoying limitations.

'Not part of POSIX' has never stopped us from using something in glib:
atomics, futexes, inotify, pipe2, libelf, to name just a few...
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Matthias Clasen-2
In reply to this post by Ryan Lortie
On Sat, Mar 21, 2015 at 1:10 AM, Ryan Lortie <[hidden email]> wrote:

> On Fri, Mar 20, 2015, at 23:33, Matthias Clasen wrote:
>> So,  you found that dup3 doesn't do what you want, and now you want to
>> throw out the baby with the bathwater and just say "I don't care
>> anymore if we leak fds" ?
>
> dup3() was a bit of a "straw that broke the camel's back" case.  I could
> point at the existence of g_unix_open_pipe() as a similarly ridiculous
> case, or many others.
>
> I'm also not impressed by the inaccurate categorisation.  I thought I
> explained fairly clearly why I believe that leaked fds will _not_ be the
> case, even without O_CLOEXEC.
>
> I was looking for some slightly more constructive arguments...

Before we can have constructive arguments, we first need to understand
what you are actually proposing. Most of your mail was an extended
whine about inadequacies of posix in general, and fd inheritance in
particular. Jasper asked the right question:

Will you accept patches that add cloexec-correctness where it is
missing, in the future ?

Are you actually suggesting that we rip out all code that is currently
doing the right thing, cloexec-wise ?
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Nirbheek Chauhan
On Sat, Mar 21, 2015 at 9:39 PM, Matthias Clasen
<[hidden email]> wrote:

> Before we can have constructive arguments, we first need to understand
> what you are actually proposing. Most of your mail was an extended
> whine about inadequacies of posix in general, and fd inheritance in
> particular. Jasper asked the right question:
>
> Will you accept patches that add cloexec-correctness where it is
> missing, in the future ?
>
> Are you actually suggesting that we rip out all code that is currently
> doing the right thing, cloexec-wise ?

From Ryan's original email:

> So: starting today I'm going to stop worrying about O_CLOEXEC being set
> on every file descriptor that GLib creates.  I'm not going to go and
> retroactively tear things out where they are already working, unless it
> would provide a substantial cleanup or fixes an actual bug.  I'm not
> just going to go around looking for #ifdefs to remove.

So, I would say no.

--
~Nirbheek Chauhan
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Chris Vine
In reply to this post by Florian Müllner-2
On Sat, 21 Mar 2015 13:47:04 +0000
Florian Müllner <[hidden email]> wrote:

> On Sat, Mar 21, 2015 at 12:31 PM Chris Vine
> <[hidden email]> wrote:
> >
> > Further, there are cases where porting to GSubprocess does not
> > actually do the job easily because (as I understand it)
> > GSubprocessFlags can be set to either close all descriptors on exec
> > other than those for stdin, stdout and stderr, or not close any
> > descriptors on exec.
> >
>
> Isn't that case covered by g_subprocess_launcher_take_fd()?

It looks as if it is.  That's good to know.

Given the restrictions on what you can do between a fork and an exec
in a multi-threaded program (and gio is multi-threaded) the
implementation of GSubprocess looks as if it must be quite clever.

Chris
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Ryan Lortie
In reply to this post by Matthias Clasen-2
On Sat, Mar 21, 2015, at 12:04, Matthias Clasen wrote:
> 'Not part of POSIX' has never stopped us from using something in glib:
> atomics, futexes, inotify, pipe2, libelf, to name just a few...

...and in each of these cases, we pay the price with some sort of
abstraction layer, #ifdef, fallback cases, etc.

Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Reply | Threaded
Open this post in threaded view
|

Re: I'm done with O_CLOEXEC

Ryan Lortie
In reply to this post by Matthias Clasen-2
On Sat, Mar 21, 2015, at 12:09, Matthias Clasen wrote:
> Are you actually suggesting that we rip out all code that is currently
> doing the right thing, cloexec-wise ?

from my original email:

"""

I'm not going to go and retroactively tear things out where they are
already working, unless it would provide a substantial cleanup or fixes
an actual bug.  I'm not just going to go around looking for #ifdefs to
remove.

"""


Cheers
_______________________________________________
gtk-devel-list mailing list
[hidden email]
https://mail.gnome.org/mailman/listinfo/gtk-devel-list
12