fork + exec; what are the possible resource leaks?
in [Unix Programming]
|
From: Chris Friesen on 19 Apr 2010 18:36 On 04/19/2010 03:50 PM, David Given wrote: > Consider a > hypothetical terminal emulator running under a multithreaded UI library > such as GTK. > > When the UI library starts up, it's going to create background threads > to do work --- it says clearly in the docs that it's going to do this, > so this isn't a problem. But once the UI library has started, the main > program now cannot safely call forkpty() and exec() to start the child > process, because one of those background threads might open a file > descriptor at the wrong time and get it propagated to the child. > > The least bad way I know of dealing with this is to use one of the > aforesaid foul hacks to close unwanted file descriptors in the child > after it's forked, before exec() is called. But these are non-portable > and not necessarily very reliable, as we've already seen... In a situation where the library is the framework for the app and is itself multithreaded, the cleanest solution would probably be for the library to provide a function for the app to call that would fork off a clean process with no leakage. Realistically, only the library has the knowledge of what is required to do this in a completely safe way. I'm no GTK programmer, but a couple minutes of poking around found the g_spawn_* interfaces which provide a supported way to create a new process. The default behaviour is to close all descriptors except stdin/stdout/stderr before calling exec(), but this can be overridden if you want them left open. Chris
From: David Schwartz on 19 Apr 2010 19:03 On Apr 19, 12:08 pm, David Given <d...(a)cowlark.com> wrote: > My understanding of the problem is that *any* call to fopen(), in a > multithreaded application, may result in a leaked file descriptor > (because even if the caller to fopen() remembers to call > fcntl(fileno(fp), F_SETFD, FD_CLOEXEC) afterwards, there's still a > window whereby another thread calling fork() might propagate the file > descriptor). > > Is this correct? > > If so, is there any actual solution? Because fopen() is not going to go > away. Install appropriate at fork handlers. Before the fork, acquire a lock. After the fork, release the lock. Call 'fopen' while holding the lock. For me, the practice is simply not to use 'fopen'. There have historically been too many problems with it, and even though most of those problems are solved now, for portability it's (IMO) best just not to use it. Bluntly, it's an ugly wrapper around 'open'. DS
From: Ersek, Laszlo on 19 Apr 2010 20:15 On Mon, 19 Apr 2010, David Given wrote: > On 19/04/10 20:39, Rainer Weikusat wrote: > [...] >> Yes. The actual solution is still (and will remain forever): DO NOT DO >> THIS. This cannot be that complicated, can it? > > So, basically, you're saying, don't use fopen()? In any multithreaded > apps? And in any *library* which might be used by a multithreaded app? s/fopen/fork, I guess. > We all know that the default for close-on-exec is wrong; are there any > *useful* strategies for dealing with it in a real-world environment? Reimplement open(), forward request to actual (or rather, next, as in RTLD_NEXT) open(), but add O_CLOEXEC. If you're lucky, fopen() / freopen() / whatever will go through open(). $ cat cloexec.c #define _GNU_SOURCE #include <stdarg.h> #include <fcntl.h> #include <dlfcn.h> static int (*real_open)(const char *, int, ...); int open(const char *path, int oflag, ...) { int ret; if (oflag & O_CREAT) { va_list ap; va_start(ap, oflag); ret = (*real_open)(path, oflag | O_CLOEXEC, va_arg(ap, mode_t)); va_end(ap); } else { ret = (*real_open)(path, oflag | O_CLOEXEC); } return ret; } static void init(void) __attribute__((constructor)); static void init(void) { *(void **)&real_open = dlsym(RTLD_NEXT, "open"); } $ gcc -fPIC -shared -o cloexec.so -Wall -Wextra cloexec.c -ldl $ strace touch testfile 2>&1 | grep O_WRONLY open("testfile", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK, 0666) = 3 $ LD_PRELOAD=./cloexec.so strace touch testfile 2>&1 | grep O_WRONLY open("testfile", O_WRONLY|O_CREAT|O_NOCTTY|O_NONBLOCK|O_CLOEXEC, 0666) = 3 Now do something similar with socket(): https://bugzilla.redhat.com/show_bug.cgi?id=443321#c10 http://www.kernel.org/doc/man-pages/online/pages/man2/socket.2.html Call pipe2() instead of pipe(): http://www.kernel.org/doc/man-pages/online/pages/man2/pipe2.2.html etc etc Cheers, lacos
From: William Ahern on 19 Apr 2010 20:19 David Given <dg(a)cowlark.com> wrote: > On 19/04/10 20:39, Rainer Weikusat wrote: > [...] > > Yes. The actual solution is still (and will remain forever): DO NOT DO > > THIS. This cannot be that complicated, can it? > So, basically, you're saying, don't use fopen()? In any multithreaded > apps? And in any *library* which might be used by a multithreaded app? > Which these days, given how popular multithreaded environments like > GNOME, web servers, and in fact any non-trivial application are, is > pretty much all code? The usual rule of thumb is not to mix threading with fork/exec. In fact, I can't think of any scenario where I would want to use fork or exec after starting some threads. Yes, it can be done, and implementions make it work, but obviously there are unresolved--perhaps unresolvable--caveats. This is an artifact of the fact that threads came along late in the history of Unix. Until very recently processes have always been the dominate parallelism paradigm, with simple concurrency handled by poll() and non-blocking I/O. This is in contrast to Windows, which is heavily multithreaded for both parallelism and concurrency; in fact, Windows had threads before it even had robust protected memory processes, IIRC. As regards any library that open descriptors and/or starts threads when it loads--that's a patently broken library in my opinion, especially if it doesn't provide any way to release those resources explicitly.
From: Ersek, Laszlo on 20 Apr 2010 15:27
On Tue, 20 Apr 2010, David Given wrote: > On 20/04/10 01:15, Ersek, Laszlo wrote: >> Now do something similar with socket(): >> Call pipe2() instead of pipe(): > Ah, but it's not me that's calling socket() or pipe() --- it's a > third-party library that's not under my control. And setting the CLOEXEC > flag for socket() as described still has a (rare, but potential) race > condition between socket() returning and the call to fcntl() immediately > afterward. No, I meant, "write a similar wrapper for pipe(), which diverts all calls to pipe2(), and write a similar wrapper for socket(), which delegates all calls to the 'next' socket() with SOCK_CLOEXEC or'd in". Basically, you'd have to "override" all functions that return new file descriptors. Some would be implemented by or-ing in a flag and delegating to the overridden function, others would be implemented by calling differently named functions. For the latter ones, you wouldn't need to call dlsym(RTLD_NEXT, "...") in the constructor function. Of course if the lib used pipe2() itself, then you'd have to override even that. Cheers, lacos |