Bug #1821

FD_CLOEXEC not set for log_access_fd and err->fd

Added by Safari over 5 years ago. Updated over 5 years ago.

Status:FixedStart date:2008-11-13
Priority:NormalDue date:
Assignee:-% Done:

100%

Category:-Estimated time:1.00 hour
Target version:-
Missing in 1.5.x:

Description

when cycling logs, CLOEXEC flag is not set.

But I have another point: in mod_cgi.c fd's 3 to 255 are closed, whereas 256 to srv->max_fds are left open.
On systems which support CLOEXEC flag, it should be set also for fd's returned by socket() (maybe pipe() dup*(), but not when dup()ing fds 0-2 when preparing for execve).

I'd do open_cloexec function which takes as parameters pathname, flags, mode as usual, but then,
if O_CLOEXEC is supported, OR O_CLOEXEC to flags.
Looks neater when there's not always that #ifdef FD_CLOEXEC etc hassle...

Same with socket_cloexec: SOCK_CLOEXEC is ORed to socket_type.

Then, when all has been fixed, on systems supporting FD_CLOEXEC the close(3..255) loop in mod_cgi.c can be skipped.
Maybe, if system does not support CLOEXEC, also cache highest fd, so you don't have to call close 4000 times only to get EBADFD, in case only a couple of fd's are actually used.

Associated revisions

Revision 2363
Added by stbuehler over 5 years ago

Use FD_CLOEXEC if possible (fixes #1821)

History

#1 Updated by Safari over 5 years ago

Caching highest fd may be hard to implement in a race-free manner...

But would be neat, since closing 4000 fd's needs two milliseconds of CPU time on P4/2.8 GHz / Linux 2.6.27.5.

#2 Updated by stbuehler over 5 years ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

Applied in changeset r2363.

#3 Updated by stbuehler over 5 years ago

Apart from mod_cgi there should be no performance issues with the "close"-loop. And if you use mod_cgi, you shouldn't care about performance anyway :)

I tried to find all places in which we forgot FD_CLOEXEC - but sometimes it just is not possible (e.g. i have no idea how sqlite works); that is why i don't want to remove the loop completely.
But 3..255 should be enough for that cases, as i think they are only used during startup. (finding/caching the highest fd is just impossible)

Also available in: Atom