From 5d0c0114fde2d1a532ca91d25331d7dd9d4b6c32 Mon Sep 17 00:00:00 2001
From: Yann Ylavic <ylavic@apache.org>
Date: Tue, 28 Jun 2016 13:48:44 +0000
Subject: [PATCH] mod_proxy: follow up to r1750392 and r1750474. Restore
 PROXY_WORKER_IS_USABLE() check in ap_proxy_connect_backend(), we must
 obviously (un)put backend in error state based on the result of the actual
 connect(), and don't change it in ap_proxy_check_backend()... APR_SUCCESS
 return by ap_proxy_check_backend(), i.e. a usable worker and an established
 connection, is enough for modules to continue w/o calling
 ap_proxy_connect_backend(), still.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1750508 13f79535-47bb-0310-9956-ffa450edef68
---
 modules/proxy/proxy_util.c | 73 ++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 7ae1d24392..52bee2098a 100644
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -2738,7 +2738,7 @@ PROXY_DECLARE(apr_status_t) ap_proxy_check_backend(const char *proxy_function,
         rv = APR_EPIPE;
     }
 
-    if (rv != APR_SUCCESS && rv != APR_ENOTSOCK) {
+    if (rv != APR_SUCCESS && conn->sock) {
         /* This clears conn->scpool (and associated data), so backup and
          * restore any ssl_hostname for this connection set earlier by
          * ap_proxy_determine_connection().
@@ -2767,36 +2767,6 @@ PROXY_DECLARE(apr_status_t) ap_proxy_check_backend(const char *proxy_function,
         }
     }
 
-    if (rv != APR_NOTFOUND) {
-        /*
-         * 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 (rv != APR_SUCCESS && rv != APR_ENOTEMPTY) {
-            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, APLOGNO(00959)
-                    "ap_proxy_connect_backend disabling worker for (%s) for %"
-                    APR_TIME_T_FMT "s",
-                    worker->s->hostname, apr_time_sec(worker->s->retry));
-            }
-        }
-        else {
-            if (worker->s->retries) {
-                /*
-                 * A worker came back. So here is where we need to
-                 * either reset all params to initial conditions or
-                 * apply some sort of aging
-                 */
-            }
-            worker->s->error_time = 0;
-            worker->s->retries = 0;
-        }
-    }
-
     return rv;
 }
 
@@ -2994,6 +2964,47 @@ PROXY_DECLARE(int) ap_proxy_connect_backend(const char *proxy_function,
         }
     }
 
+    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 (rv != APR_SUCCESS) {
+            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, APLOGNO(00959)
+                    "ap_proxy_connect_backend disabling worker for (%s) for %"
+                    APR_TIME_T_FMT "s",
+                    worker->s->hostname, apr_time_sec(worker->s->retry));
+            }
+        }
+        else {
+            if (worker->s->retries) {
+                /*
+                 * A worker came back. So here is where we need to
+                 * either reset all params to initial conditions or
+                 * apply some sort of aging
+                 */
+            }
+            worker->s->error_time = 0;
+            worker->s->retries = 0;
+        }
+    }
+    else {
+        /*
+         * 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 (rv == APR_SUCCESS) {
+            socket_cleanup(conn);
+        }
+        rv = APR_NOTFOUND;
+    }
+
     return rv == APR_SUCCESS ? OK : DECLINED;
 }