mod_proxy: Fix a race condition that caused a failed worker to be retried

before the retry period is over

Backports: r1664709, r1697323
Submitted by: rpluem
Reviewed by: wrowe, ylavic

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

diff --git a/CHANGES b/CHANGES
index 3a8319c2d7bd83e0efc8e6f555d19a3dfbcc970e..23d4cee690e72c91a4e39211f09f1b46f749f148 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,13 +1,20 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.2.32
 
-  *) mime.types: add common extension "m4a" for MPEG 4 Audio.
-     PR 57895 [Dylan Millikin <dylan.millikin gmail.com>]
+  *) mod_proxy: Fix a regression with 2.2.31 that caused inherited workers to
+     use a different scoreboard slot then the original one.  PR 58267.
+     [Ruediger Pluem]
+
+  *) mod_proxy: Fix a race condition that caused a failed worker to be retried
+     before the retry period is over. [Ruediger Pluem]
 
   *) mod_proxy: don't recyle backend announced "Connection: close" connections
      to avoid reusing it should the close be effective after some new request
      is ready to be sent.  [Yann Ylavic]
 
+  *) mime.types: add common extension "m4a" for MPEG 4 Audio.
+     PR 57895 [Dylan Millikin <dylan.millikin gmail.com>]
+
   *) mod_substitute: Allow to configure the patterns merge order with the new
      SubstituteInheritBefore on|off directive.  PR 57641
      [Marc.Stern <Marc.Stern approach.be>, Yann Ylavic, William Rowe]
@@ -16,10 +23,6 @@ Changes with Apache 2.2.32
      failures under Visual Studio 2015 and other mismatched MSVCRT flavors.
      PR59630 [Jan Ehrhardt <phpdev ehrhardt.nl>]
 
-  *) mod_proxy: Fix a regression with 2.2.31 that caused inherited workers to
-     use a different scoreboard slot then the original one.  PR 58267.
-     [Ruediger Pluem]
-
 Changes with Apache 2.2.31
 
   *) Correct win32 build issues for mod_proxy exports, OpenSSL 1.0.x headers.

diff --git a/STATUS b/STATUS
index e5d14e7bf19c958c2d7275f3d7d1c1af00f038f3..739ea20a583e263e68d54bdd82b46f7e1da6873a 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -103,14 +103,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_proxy: Fix a race condition that caused a failed worker to be retried
-     before the retry period is over
-      Trunk version of patch:
-         http://svn.apache.org/r1664709
-         http://svn.apache.org/r1697323
-      Backport version for 2.2.x of patch:
-         http://people.apache.org/~rpluem/patches/proxy_race_retry_2.2.x_v2.diff
-      +1: rpluem, wrowe, ylavic
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 66bc42be1f4924d3143888aff8ee4767c51cba43..985a60974d98fa60f7e881f584abbf99c1fb6772 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -2647,25 +2647,39 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function,
 
         connected    = 1;
     }
-    /*
-     * Put the entire worker to error state if
-     * the PROXY_WORKER_IGNORE_ERRORS flag is not set.
-     * Altrough some connections may be alive
-     * no further connections to the worker could be made
-     */
-    if (!connected && PROXY_WORKER_IS_USABLE(worker) &&
-        !(worker->s->status & PROXY_WORKER_IGNORE_ERRORS)) {
-        worker->s->error_time = apr_time_now();
-        worker->s->status |= PROXY_WORKER_IN_ERROR;
-        ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
-            "ap_proxy_connect_backend disabling worker for (%s)",
-            worker->hostname);
+    if (PROXY_WORKER_IS_USABLE(worker)) {
+        /*
+         * Put the entire worker to error state if
+         * the PROXY_WORKER_IGNORE_ERRORS flag is not set.
+         * Although some connections may be alive
+         * no further connections to the worker could be made
+         */
+        if (!connected) {
+            if (!(worker->s->status & PROXY_WORKER_IGNORE_ERRORS)) {
+                worker->s->error_time = apr_time_now();
+                worker->s->status |= PROXY_WORKER_IN_ERROR;
+                ap_log_error(APLOG_MARK, APLOG_ERR, 0, s,
+                    "ap_proxy_connect_backend disabling worker for (%s)",
+                    worker->hostname);
+            }
+        }
+        else {
+            worker->s->error_time = 0;
+            worker->s->retries = 0;
+        }
+        return connected ? OK : DECLINED;
     }
     else {
-        worker->s->error_time = 0;
-        worker->s->retries = 0;
+        /*
+         * The worker is in error likely done by a different thread / process
+         * e.g. for a timeout or bad status. We should respect this and should
+         * not continue with a connection via this worker even if we got one.
+         */
+        if (connected) {
+            socket_cleanup(conn);
+        }
+        return DECLINED;
     }
-    return connected ? OK : DECLINED;
 }
 
 PROXY_DECLARE(int) ap_proxy_connection_create(const char *proxy_function,