apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: svn commit: r794485 - /apr/apr/trunk/configure.in
Date Thu, 16 Jul 2009 08:27:39 GMT
On Thu, Jul 16, 2009 at 11:42:52AM +1000, Bojan Smojver wrote:
> On Thu, 2009-07-16 at 01:39 +0000, bojan@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,

Mime
View raw message