On Thu, Jul 16, 2009 at 11:42:52AM +1000, Bojan Smojver wrote:
> On Thu, 2009-07-16 at 01:39 +0000, bo...@apache.org wrote:
> > Use more elaborate checks for epoll_create1, dup3 and accept4.
>
> This requires review folks. There is code in the tests that can
> potentially hang the configure process. It does work on my Fedora 11 box
> (which has the functions) and on RHEL4 (which doesn't have them), but it
> would be really great to test this on other platforms, especially other
> Linux flavours.
The accept4() configure test looks a bit overcomplicated; it would be
possible to simplify a bit to avoid the fork etc, by creating a socket
with SOCK_NONBLOCK set, and simply calling accept4() and presuming the
syscall is wired up if it returns EAGAIN.
But, on the issue of runtime/build-time feature detection:
In the past I have argued we should never do runtime platform feature
detection, i.e., if accept4() works at build-time, we can presume it
works at run-time. I think I need to soften that position now; use of a
modern userspace on an older kernel seems to be very widespread in Xen
hosting sites. I shipped socket(,SOCK_CLOEXEC) support in neon and got
lots of complaints, from users across the spectrum of distributions.
So I fear we should probably do the same thing in APR. Any other
opinions? (Debianites, feel free to send me the hate mail now ;)
It would simplify the configure script, e.g. for accept4 support we
could use simply AC_CHECK_FUNCS and change apr_socket_accept something
like this (completely untested):
Thoughts?
Index: network_io/unix/sockets.c
===================================================================
--- network_io/unix/sockets.c (revision 790537)
+++ network_io/unix/sockets.c (working copy)
@@ -230,14 +230,21 @@
apr_status_t apr_socket_accept(apr_socket_t **new, apr_socket_t *sock,
apr_pool_t *connection_context)
{
- int s;
+ int s, flags;
apr_sockaddr_t sa;
sa.salen = sizeof(sa.sa);
#ifdef HAVE_ACCEPT4
+ /* Attempt to use accept4() but fall back on accept(). */
+ flags = FD_CLOEXEC;
s = accept4(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen,
SOCK_CLOEXEC);
+ if (s < 0 && errno == ENOSYS) {
+ flags = 0;
+ s = accept(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen);
+ }
#else
+ flags = 0;
s = accept(sock->socketdes, (struct sockaddr *)&sa.sa, &sa.salen);
#endif
@@ -321,10 +328,10 @@
(*new)->local_interface_unknown = 1;
}
-#ifndef HAVE_ACCEPT4
- {
- int flags;
-
+ /* Toggle the CLOEXEC bit on. Note that this will be done either
+ * if no accept4() support was present, or, if it was present but
+ * failed with ENOSYS. */
+ if (flags == 0) {
if ((flags = fcntl((*new)->socketdes, F_GETFD)) == -1)
return errno;
@@ -332,7 +339,6 @@
if (fcntl((*new)->socketdes, F_SETFD, flags) == -1)
return errno;
}
-#endif
(*new)->inherit = 0;
apr_pool_cleanup_register((*new)->pool, (void *)(*new), socket_cleanup,
