[PATCH] ev: fix epoll_init fd leak

[Christian Parpart]

On Tue, Nov 1, 2011 at 1:16 AM, Marc Lehmann <schmorp at schmorp.de> wrote:

> On Mon, Oct 31, 2011 at 09:07:39PM +0100, Jonathan Neuschäfer <
> j.neuschaefer at gmx.net> wrote:
> > > 0 is, in fact, not a valid file descriptor in this case, as it is
> stdin,
> > > and an epoll fd is not having the properties stdin has - althoguh it'S
> > > much betterto have an epoll fd there than, say, your shadow file :)
> >
> > As this is not exactly the same as checking for a simple error, I think
> > this line needs a comment why it checks for <= 0 and not < 0.
>
> The check for <= 0 is inconsistent and incomplete - libev should either
> check for <= 2 in all such tests, which gives best coverage of system-fds,
> or always < 0. Alternatively, it could do what urxvt does and fix up fds
> 0..2 on init, with warning, as it seems to be a common bug.
>
> But this is a quality issue - libev doesn't really give a useful
> diagnostic in this case, although no harm will be done as it is (libev
> does not and cannot guarantee atomicity w.r.t. concurrent exec, and libev
> will work under these conditions).
>
> The real issue was and is the broken app that messes with system fds. It
> needs fixing in any case.


I do think a little bit different here. Think of a server app. The app:
1.) first reads config file,
2.) populates internal data structures out of it,
3.) cleans up its environment (including closing inherited fds, including
0,1,2)
4.) initializes its own subsystem, including epoll_create(libev) and its
own log targets and listener sockets
5.) runs the loop until exit.

This is not a bad behaviour.
closing all inherited fds right before starting to allocate its intended
ones is even a really
sane approach to prevent stale fds from the parent process. (again, we talk
about a server app).
now, if the server app would *first* create the log targets (open log
files, syslog sockets, ...)
and *then* initializing libev, then libev's epoll_create() invokation is
fine.
but dare to do it the other way around (as shown above), then your test
fails, even though
epoll_create DID NOT fail. that is a bug in libev, no matter how you turn
it and the
app developer has to know that buggy part of libev to not run into it (e.g.
by making sure fd 0
is in use, not taking into account fd 1 and 2).

Really, unless you do not know what you're doing, this is the sanest
approach, because you
ensure to make use of allmost all resources are at your service.
While you should never come even close to your software limits in a peak
load, it would be a huge
waste to live in spirit of yours (just keeping fds around for nothing,
knowing, that libev is the
only void dependency that needs them to properly initialize, wtf).

p.s.: you wanted a software-example, well, I once worked at a company where
resources (like fds)
and high performance where treated very precise, keeping fds around for
nothing would have been
a joke, though, sure, you might work around libev's behaviour.

Best regards,
Christian Parpart.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.schmorp.de/pipermail/libev/attachments/20111102/0e435040/attachment.html>