Allow modules that add sockets to the ap_listeners list to

define the function that should be used to accept on that
socket.  Each MPM can define their own function to use for
the accept function with the MPM_ACCEPT_FUNC macro.  This
also abstracts out all of the Unix accept error handling
logic, which has become out of synch across Unix MPMs.

The code flow is much easier now for different transports:

1)  During pre-config, post-config or while parsing the config
    file, add a socket to the ap_listeners list, making sure to
    define an accept function at the same time.

2)  MPMs find the correct listener, and call the accept function
    that was defined in step 1.

3)  That accept function returns a void pointer, which is passed
    to the create_connection hook.

4)  create_connection adds the correct low-level filters.


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@91916 13f79535-47bb-0310-9956-ffa450edef68

8 files changed, 168 insertions, 131 deletions

diff --git a/CHANGES b/CHANGES
index 7d6770a900..c36f9cfe2f 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,13 @@
 Changes with Apache 2.0.29-dev
 
+  *) Allow modules that add sockets to the ap_listeners list to
+     define the function that should be used to accept on that
+     socket.  Each MPM can define their own function to use for
+     the accept function with the MPM_ACCEPT_FUNC macro.  This
+     also abstracts out all of the Unix accept error handling
+     logic, which has become out of synch across Unix MPMs.
+     [Ryan Bloom]
+
   *) Fix a bug which would cause the response headers to be omitted
      when sending a negotiated ErrorDocument because the required
      filters were attached to the wrong request_rec.
diff --git a/include/ap_listen.h b/include/ap_listen.h
index 278e84e530..7cb2c3c18e 100644
--- a/include/ap_listen.h
+++ b/include/ap_listen.h
@@ -58,13 +58,13 @@
 #include "apr_network_io.h"
 #include "httpd.h"
 #include "http_config.h"
-#include "mpm.h"
 
 /**
  * @package Apache Listeners Library
  */
 
 typedef struct ap_listen_rec ap_listen_rec;
+typedef apr_status_t (*accept_function)(void **csd, ap_listen_rec *lr, apr_pool_t *ptrans);
 
 /**
  * Apache's listeners record.  These are used in the Multi-Processing Modules
@@ -84,6 +84,10 @@ struct ap_listen_rec {
      */
     apr_sockaddr_t *bind_addr;
     /**
+     * The accept function for this socket
+     */
+    accept_function accept_func;
+    /**
      * Is this socket currently active 
      */
     int active;
diff --git a/include/ap_mpm.h b/include/ap_mpm.h
index d223984cc5..96c24a8904 100644
--- a/include/ap_mpm.h
+++ b/include/ap_mpm.h
@@ -55,6 +55,8 @@
 #ifndef AP_MMN_H
 #define AP_MMN_H
 
+#include "apr_thread_proc.h"
+
 /**
  * @package Multi-Processing Module library
  */
diff --git a/os/unix/unixd.c b/os/unix/unixd.c
index e705988191..673a1cfd9b 100644
--- a/os/unix/unixd.c
+++ b/os/unix/unixd.c
@@ -441,3 +441,126 @@ AP_DECLARE(apr_status_t) unixd_set_proc_mutex_perms(apr_proc_mutex_t *pmutex)
     return APR_SUCCESS;
 }
 
+AP_DECLARE(apr_status_t) unixd_accept(void **accepted, ap_listen_rec *lr,
+                                        apr_pool_t *ptrans)
+{
+    apr_socket_t *csd;
+    apr_status_t status;
+    int sockdes;
+
+    status = apr_accept(&csd, lr->sd, ptrans);
+    if (status == APR_SUCCESS) { 
+        *accepted = csd;
+        apr_os_sock_get(&sockdes, csd);
+        if (sockdes >= FD_SETSIZE) {
+            ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING, 0, NULL,
+                         "new file descriptor %d is too large; you probably need "
+                         "to rebuild Apache with a larger FD_SETSIZE "
+                         "(currently %d)",
+                         sockdes, FD_SETSIZE);
+            apr_socket_close(csd);
+            return APR_EINTR;
+        } 
+#ifdef TPF
+        if (sockdes == 0) {                  /* 0 is invalid socket for TPF */
+            return APR_EINTR;
+        }
+#endif
+        return status;
+    }
+
+    if (APR_STATUS_IS_EINTR(status)) {
+        return status;
+    }
+    /* Our old behaviour here was to continue after accept()
+     * errors.  But this leads us into lots of troubles
+     * because most of the errors are quite fatal.  For
+     * example, EMFILE can be caused by slow descriptor
+     * leaks (say in a 3rd party module, or libc).  It's
+     * foolish for us to continue after an EMFILE.  We also
+     * seem to tickle kernel bugs on some platforms which
+     * lead to never-ending loops here.  So it seems best
+     * to just exit in most cases.
+     */
+    switch (status) {
+#ifdef EPROTO
+        /* EPROTO on certain older kernels really means
+         * ECONNABORTED, so we need to ignore it for them.
+         * See discussion in new-httpd archives nh.9701
+         * search for EPROTO.
+         *
+         * Also see nh.9603, search for EPROTO:
+         * There is potentially a bug in Solaris 2.x x<6,
+         * and other boxes that implement tcp sockets in
+         * userland (i.e. on top of STREAMS).  On these
+         * systems, EPROTO can actually result in a fatal
+         * loop.  See PR#981 for example.  It's hard to
+         * handle both uses of EPROTO.
+         */
+        case EPROTO:
+#endif
+#ifdef ECONNABORTED
+        case ECONNABORTED:
+#endif
+        /* Linux generates the rest of these, other tcp
+         * stacks (i.e. bsd) tend to hide them behind
+         * getsockopt() interfaces.  They occur when
+         * the net goes sour or the client disconnects
+         * after the three-way handshake has been done
+         * in the kernel but before userland has picked
+         * up the socket.
+         */
+#ifdef ECONNRESET
+        case ECONNRESET:
+#endif
+#ifdef ETIMEDOUT
+        case ETIMEDOUT:
+#endif
+#ifdef EHOSTUNREACH
+        case EHOSTUNREACH:
+#endif
+#ifdef ENETUNREACH
+        case ENETUNREACH:
+#endif
+            break;
+#ifdef ENETDOWN
+        case ENETDOWN:
+            /*
+             * When the network layer has been shut down, there
+             * is not much use in simply exiting: the parent
+             * would simply re-create us (and we'd fail again).
+             * Use the CHILDFATAL code to tear the server down.
+             * @@@ Martin's idea for possible improvement:
+             * A different approach would be to define
+             * a new APEXIT_NETDOWN exit code, the reception
+             * of which would make the parent shutdown all
+             * children, then idle-loop until it detected that
+             * the network is up again, and restart the children.
+             * Ben Hyde noted that temporary ENETDOWN situations
+             * occur in mobile IP.
+             */
+            ap_log_error(APLOG_MARK, APLOG_EMERG, status, ap_server_conf,
+                         "apr_accept: giving up.");
+            return APR_EGENERAL;
+#endif /*ENETDOWN*/
+
+#ifdef TPF
+        case EINACT:
+            ap_log_error(APLOG_MARK, APLOG_EMERG, status, ap_server_conf,
+                         "offload device inactive");
+            return APR_EGENERAL;
+            break;
+        default:
+            ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, ap_server_conf,
+                         "select/accept error (%d)", status);
+            return APR_EGENERAL;
+#else
+        default:
+            ap_log_error(APLOG_MARK, APLOG_ERR, status, ap_server_conf,
+                         "apr_accept: (client socket)");
+            return APR_EGENERAL;
+#endif
+    }
+    return status;
+}
+
diff --git a/os/unix/unixd.h b/os/unix/unixd.h
index 35adbc147e..ef906feca4 100644
--- a/os/unix/unixd.h
+++ b/os/unix/unixd.h
@@ -61,6 +61,7 @@
 
 #include "httpd.h"
 #include "http_config.h"
+#include "ap_listen.h"
 #ifdef HAVE_SYS_TIME_H
 #include <sys/time.h>
 #endif
@@ -120,6 +121,7 @@ AP_DECLARE(void) unixd_set_rlimit(cmd_parms *cmd, struct rlimit **plimit,
 #endif
 AP_DECLARE(apr_status_t) unixd_set_lock_perms(apr_lock_t *lock);
 AP_DECLARE(apr_status_t) unixd_set_proc_mutex_perms(apr_proc_mutex_t *pmutex);
+AP_DECLARE(apr_status_t) unixd_accept(void **accepted, ap_listen_rec *lr, apr_pool_t *ptrans);
 
 #ifdef HAVE_KILLPG
 #define unixd_killpg(x, y)	(killpg ((x), (y)))
diff --git a/server/listen.c b/server/listen.c
index 6a6e919a7f..35bab34044 100644
--- a/server/listen.c
+++ b/server/listen.c
@@ -166,6 +166,11 @@ static apr_status_t make_sock(apr_pool_t *p, ap_listen_rec *server)
 
     server->sd = s;
     server->active = 1;
+#ifdef MPM_ACCEPT_FUNC
+    server->accept_func = MPM_ACCEPT_FUNC;
+#else
+    server->accept_func = NULL;
+#endif
     return APR_SUCCESS;
 }
 
diff --git a/server/mpm/prefork/mpm.h b/server/mpm/prefork/mpm.h
index 32e5512255..6b54f9891e 100644
--- a/server/mpm/prefork/mpm.h
+++ b/server/mpm/prefork/mpm.h
@@ -83,6 +83,7 @@
 #define MPM_SYNC_CHILD_TABLE() (ap_sync_scoreboard_image())
 #define MPM_CHILD_PID(i) (ap_scoreboard_image->parent[i].pid)
 #define MPM_NOTE_CHILD_KILLED(i) (MPM_CHILD_PID(i) = 0)
+#define MPM_ACCEPT_FUNC unixd_accept
 
 extern int ap_threads_per_child;
 extern int ap_max_daemons_limit;
diff --git a/server/mpm/prefork/prefork.c b/server/mpm/prefork/prefork.c
index 2e05f31a8e..83c15d33e7 100644
--- a/server/mpm/prefork/prefork.c
+++ b/server/mpm/prefork/prefork.c
@@ -527,10 +527,9 @@ static void set_signals(void)
  * they are really private to child_main.
  */
 
-static apr_socket_t *csd;
 static int requests_this_child;
 static int num_listensocks = 0;
-static apr_socket_t **listensocks;
+static ap_listen_rec *listensocks;
 
 int ap_graceful_stop_signalled(void)
 {
@@ -544,11 +543,12 @@ static void child_main(int child_num_arg)
     apr_pool_t *ptrans;
     conn_rec *current_conn;
     apr_status_t status = APR_EINIT;
-    int sockdes, i;
+    int i;
     ap_listen_rec *lr;
     int curr_pollfd, last_pollfd = 0;
     apr_pollfd_t *pollset;
-    apr_socket_t *sd;
+    int offset;
+    void *csd;
 
     my_child_num = child_num_arg;
     ap_my_pid = getpid();
@@ -579,12 +579,14 @@ static void child_main(int child_num_arg)
     /* Set up the pollfd array */
     listensocks = apr_pcalloc(pchild,
                             sizeof(*listensocks) * (num_listensocks));
-    for (lr = ap_listeners, i = 0; i < num_listensocks; lr = lr->next, i++)
-        listensocks[i]=lr->sd;
+    for (lr = ap_listeners, i = 0; i < num_listensocks; lr = lr->next, i++) {
+        listensocks[i].accept_func = lr->accept_func;
+        listensocks[i].sd = lr->sd;
+    }
 
     apr_poll_setup(&pollset, num_listensocks, pchild);
     for (i = 0; i < num_listensocks; i++)
-        apr_poll_socket_add(pollset, listensocks[i], APR_POLLIN);
+        apr_poll_socket_add(pollset, listensocks[i].sd, APR_POLLIN);
 
     while (!die_now) {
 	/*
@@ -630,7 +632,7 @@ static void child_main(int child_num_arg)
     	        clean_child_exit(1);
             }
             if (num_listensocks == 1) {
-                sd = ap_listeners->sd;
+                offset = 0;
                 goto got_fd;
             }
             else {
@@ -642,124 +644,34 @@ static void child_main(int child_num_arg)
                         curr_pollfd = 0;
                     }
                     /* XXX: Should we check for POLLERR? */
-                    apr_poll_revents_get(&event, listensocks[curr_pollfd], pollset);
+                    apr_poll_revents_get(&event, listensocks[curr_pollfd].sd, pollset);
                     if (event & APR_POLLIN) {
                         last_pollfd = curr_pollfd;
-                        sd=listensocks[curr_pollfd];
+                        offset = curr_pollfd;
                         goto got_fd;
                     }
                 } while (curr_pollfd != last_pollfd);
             }
 
             continue;
+        }
     got_fd:
 	/* if we accept() something we don't want to die, so we have to
 	 * defer the exit
 	 */
 	for (;;) {
             ap_sync_scoreboard_image();
-	    status = apr_accept(&csd, sd, ptrans);
-   	    if (status == APR_SUCCESS || !APR_STATUS_IS_EINTR(status))
-	        break;
-	    }
-
-	    if (status == APR_SUCCESS)
-		break;		/* We have a socket ready for reading */
-	    else {
-		/* Our old behaviour here was to continue after accept()
-		 * errors.  But this leads us into lots of troubles
-		 * because most of the errors are quite fatal.  For
-		 * example, EMFILE can be caused by slow descriptor
-		 * leaks (say in a 3rd party module, or libc).  It's
-		 * foolish for us to continue after an EMFILE.  We also
-		 * seem to tickle kernel bugs on some platforms which
-		 * lead to never-ending loops here.  So it seems best
-		 * to just exit in most cases.
-		 */
-                switch (status) {
-#ifdef EPROTO
-		    /* EPROTO on certain older kernels really means
-		     * ECONNABORTED, so we need to ignore it for them.
-		     * See discussion in new-httpd archives nh.9701
-		     * search for EPROTO.
-		     *
-		     * Also see nh.9603, search for EPROTO:
-		     * There is potentially a bug in Solaris 2.x x<6,
-		     * and other boxes that implement tcp sockets in
-		     * userland (i.e. on top of STREAMS).  On these
-		     * systems, EPROTO can actually result in a fatal
-		     * loop.  See PR#981 for example.  It's hard to
-		     * handle both uses of EPROTO.
-		     */
-                case EPROTO:
-#endif
-#ifdef ECONNABORTED
-                case ECONNABORTED:
-#endif
-		    /* Linux generates the rest of these, other tcp
-		     * stacks (i.e. bsd) tend to hide them behind
-		     * getsockopt() interfaces.  They occur when
-		     * the net goes sour or the client disconnects
-		     * after the three-way handshake has been done
-		     * in the kernel but before userland has picked
-		     * up the socket.
-		     */
-#ifdef ECONNRESET
-                case ECONNRESET:
-#endif
-#ifdef ETIMEDOUT
-                case ETIMEDOUT:
-#endif
-#ifdef EHOSTUNREACH
-		case EHOSTUNREACH:
-#endif
-#ifdef ENETUNREACH
-		case ENETUNREACH:
-#endif
-                    break;
-#ifdef ENETDOWN
-		case ENETDOWN:
-		     /*
-		      * When the network layer has been shut down, there
-		      * is not much use in simply exiting: the parent
-		      * would simply re-create us (and we'd fail again).
-		      * Use the CHILDFATAL code to tear the server down.
-		      * @@@ Martin's idea for possible improvement:
-		      * A different approach would be to define
-		      * a new APEXIT_NETDOWN exit code, the reception
-		      * of which would make the parent shutdown all
-		      * children, then idle-loop until it detected that
-		      * the network is up again, and restart the children.
-		      * Ben Hyde noted that temporary ENETDOWN situations
-		      * occur in mobile IP.
-		      */
-		    ap_log_error(APLOG_MARK, APLOG_EMERG, status, ap_server_conf,
-			"apr_accept: giving up.");
-		    clean_child_exit(APEXIT_CHILDFATAL);
-#endif /*ENETDOWN*/
-
-#ifdef TPF
-		case EINACT:
-		    ap_log_error(APLOG_MARK, APLOG_EMERG, status, ap_server_conf,
-			"offload device inactive");
-		    clean_child_exit(APEXIT_CHILDFATAL);
-		    break;
-		default:
-		    ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_ERR, 0, ap_server_conf,
-			"select/accept error (%d)", status);
-		    clean_child_exit(APEXIT_CHILDFATAL);
-#else
-		default:
-		    ap_log_error(APLOG_MARK, APLOG_ERR, status, ap_server_conf,
-				"apr_accept: (client socket)");
-		    clean_child_exit(1);
-#endif
-		}
-	    }
+            status = listensocks[offset].accept_func(&csd, 
+                                       &listensocks[offset], ptrans);
 
+            if (status == APR_SUCCESS) {
+                break;
+            }
+            if (status == APR_EGENERAL) {
+                clean_child_exit(APEXIT_CHILDFATAL);
+            }
             ap_sync_scoreboard_image();
-	}
-
+        }
 	SAFE_ACCEPT(accept_mutex_off());	/* unlock after "accept" */
 
 	/*
@@ -767,26 +679,6 @@ static void child_main(int child_num_arg)
 	 * socket options, file descriptors, and read/write buffers.
 	 */
 
-        apr_os_sock_get(&sockdes, csd);
-
-        if (sockdes >= FD_SETSIZE) {
-            ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING, 0, NULL,
-                         "new file descriptor %d is too large; you probably need "
-                         "to rebuild Apache with a larger FD_SETSIZE "
-                         "(currently %d)", 
-                         sockdes, FD_SETSIZE);
-	    apr_socket_close(csd);
-            ap_sync_scoreboard_image();
-	    continue;
-        }
-
-#ifdef TPF
-	if (sockdes == 0) {                  /* 0 is invalid socket for TPF */
-	    ap_sync_scoreboard_image();
-            continue;
-        }
-#endif
-
 	current_conn = ap_run_create_connection(ptrans, ap_server_conf, csd, my_child_num);
         if (current_conn) {
             ap_process_connection(current_conn);