Merge r1789220, r1792675 from trunk:

core: Disallow multiple Listen on the same IP:port when listener buckets
are configured (ListenCoresBucketsRatio > 0), consistently with the single
bucket case (default), thus fixing the leak of the corresponding socket
descriptors on graceful restart.



Follow up to r1789220.
Document the implicit behaviour of ListenCoresBucketsRatio when multiple
Listen-ers are configured on the same IP:port.


Submitted by: ylavic
Reviewed by: ylavic, jim, jorton


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1805221 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 59 insertions, 26 deletions

diff --git a/CHANGES b/CHANGES
index 8ec8b58729..e0d55ce253 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,11 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.4.28
 
+  *) core: Disallow multiple Listen on the same IP:port when listener buckets
+     are configured (ListenCoresBucketsRatio > 0), consistently with the single
+     bucket case (default), thus avoiding the leak of the corresponding socket
+     descriptors on graceful restart.  [Yann Ylavic]
+
   *) event: Avoid listener periodic wake ups by using the pollset wake-ability
      when available.  PR 57399.  [Yann Ylavic, Luca Toscano]
 
diff --git a/STATUS b/STATUS
index 73ff76b7b6..e812805cb9 100644
--- a/STATUS
+++ b/STATUS
@@ -116,15 +116,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) core: Disallow multiple Listen on the same IP:port when listener buckets
-     are configured (ListenCoresBucketsRatio > 0), consistently with the single
-     bucket case (default), thus avoiding the leak of the corresponding socket
-     descriptors on graceful restart.
-     trunk patch: http://svn.apache.org/r1789220
-                  http://svn.apache.org/r1792675
-     2.4.x patch: trunk works (modulo CHANGES)
-     +1: ylavic, jim, jorton, 
-
    *) prefork: This fixes race-condition in signals handling when httpd is
                runnning on foreground and user hits ctrl+c
      trunk patch: http://svn.apache.org/r1618555
diff --git a/docs/manual/mod/mpm_common.xml b/docs/manual/mod/mpm_common.xml
index 1da81bcd5e..08e5a3e5b2 100644
--- a/docs/manual/mod/mpm_common.xml
+++ b/docs/manual/mod/mpm_common.xml
@@ -302,6 +302,27 @@ in *BSDs.</compatibility>
     <directive module="mpm_common">MaxSpareThreads</directive> lower bound values.
     The number of children processes needs to be a multiple of the number 
     of buckets to optimally accept connections.</p>
+
+    <note>
+    <title>Multiple <directive>Listen</directive>ers or Apache HTTP servers on
+    the same IP address and port</title>
+    <p>Setting the <code>SO_REUSEPORT</code> option on the listening socket(s)
+    consequently allows multiple processes (sharing the same <code>EUID</code>,
+    e.g. <code>root</code>) to bind to the the same IP address and port,
+    without the binding error raised by the system in the usual case.</p>
+    <p>This also means that multiple instances of Apache httpd configured on a
+    same <code>IP:port</code> and with a positive <directive>ListenCoresBucketsRatio</directive>
+    would start without an error too, and then run with incoming connections
+    evenly distributed accross both instances (this is NOT a recommendation or
+    a sensible usage in any case, but just a notice that it would prevent such
+    possible issues to be detected).</p>
+    <p>Within the same instance, Apache httpd will check and fail to start if
+    multiple <directive>Listen</directive> directives on the exact same IP (or
+    hostname) and port are configured, thus avoiding the creation of some
+    duplicated buckets which would be useless and kill performances.  However
+    it can't (and won't try harder to) catch all the possible overlapping cases
+    (like a hostname resolving to an IP used elsewhere).</p>
+    </note>
 </usage>
 </directivesynopsis>
 
diff --git a/server/listen.c b/server/listen.c
index d44c7c9f44..a8e9e6f6e3 100644
--- a/server/listen.c
+++ b/server/listen.c
@@ -277,18 +277,14 @@ static apr_status_t close_listeners_on_exec(void *v)
     return APR_SUCCESS;
 }
 
-static const char *alloc_listener(process_rec *process, char *addr,
-                                  apr_port_t port, const char* proto,
-                                  void *slave)
+static int find_listeners(ap_listen_rec **from, ap_listen_rec **to,
+                          const char *addr, apr_port_t port)
 {
-    ap_listen_rec **walk, *last;
-    apr_status_t status;
-    apr_sockaddr_t *sa;
-    int found_listener = 0;
+    int found = 0;
+
+    while (*from) {
+        apr_sockaddr_t *sa = (*from)->bind_addr;
 
-    /* see if we've got an old listener for this address:port */
-    for (walk = &old_listeners; *walk;) {
-        sa = (*walk)->bind_addr;
         /* Some listeners are not real so they will not have a bind_addr. */
         if (sa) {
             ap_listen_rec *new;
@@ -301,19 +297,39 @@ static const char *alloc_listener(process_rec *process, char *addr,
             if (port == oldport &&
                 ((!addr && !sa->hostname) ||
                  ((addr && sa->hostname) && !strcmp(sa->hostname, addr)))) {
-                new = *walk;
-                *walk = new->next;
-                new->next = ap_listeners;
-                ap_listeners = new;
-                found_listener = 1;
+                found = 1;
+                if (!to) {
+                    break;
+                }
+                new = *from;
+                *from = new->next;
+                new->next = *to;
+                *to = new;
                 continue;
             }
         }
 
-        walk = &(*walk)->next;
+        from = &(*from)->next;
+    }
+
+    return found;
+}
+
+static const char *alloc_listener(process_rec *process, const char *addr,
+                                  apr_port_t port, const char* proto,
+                                  void *slave)
+{
+    ap_listen_rec *last;
+    apr_status_t status;
+    apr_sockaddr_t *sa;
+
+    /* see if we've got a listener for this address:port, which is an error */
+    if (find_listeners(&ap_listeners, NULL, addr, port)) {
+        return "Cannot define multiple Listeners on the same IP:port";
     }
 
-    if (found_listener) {
+    /* see if we've got an old listener for this address:port */
+    if (find_listeners(&old_listeners, &ap_listeners, addr, port)) {
         if (ap_listeners->slave != slave) {
             return "Cannot define a slave on the same IP:port as a Listener";
         }