Re: I'm done with O_CLOEXEC
hi, On Sat, Mar 21, 2015, at 01:59, Jürg Billeter wrote: > 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. This is probably the best counter-argument so far: since we all agree that the inherit-by-default behaviour is silly, we should try as much as possible to mitigate it. I agree with that point, but the main thrust of the argument that I was trying to make originally (and somewhat detracted from with my ranting) is: - there is often a high price to pay for "doing the right thing"; and - it's not actually necessary Sure, it's conceivable (and even likely) that there is code that isn't up to snuff, but that's sort of the point of this thread. I want to shift responsibility for "getting this right" from the innumerably many places that we create fds to the very few places that we launch subprocesses. And by the way: the "high price" that we pay is not just in cases where we need to implement one codepath for Linux and a fallback for other platforms inside of GLib. If you fully buy into the O_CLOEXEC mantra, then it means that every single person who casually calls open() (even in application code) needs to take care to get it right, for fear that some naive library is doing fork()/exec() in another thread. Cheers ___ gtk-devel-list mailing list gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
On Sat, 21 Mar 2015 13:47:04 + Florian Müllner wrote: > On Sat, Mar 21, 2015 at 12:31 PM Chris Vine > 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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
On Sat, Mar 21, 2015 at 9:39 PM, Matthias Clasen 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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
On Sat, Mar 21, 2015 at 1:10 AM, Ryan Lortie 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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
On Fri, Mar 20, 2015 at 4:43 PM, Ryan Lortie 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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
On Sat, Mar 21, 2015 at 12:31 PM Chris Vine 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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
Re: I'm done with O_CLOEXEC
On Sat, 21 Mar 2015 06:59:41 +0100 Jürg Billeter 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 gtk-devel-list@gnome.org https://mail.gnome.org/mailman/listinfo/gtk-devel-list
