.. is meant to do a simple TCP readability check on the connection,
using ap_proxy_is_socket_connected().
This is trunk only AFAICT.

However all our proxy modules handle this just after calling
ap_proxy_connect_backend() which already calls
ap_proxy_is_socket_connected() for the same purpose.

Couldn't we simply remove this ping_timeout < 0 code (eg. attached patch)?

Regards,
Yann.
Index: modules/proxy/mod_proxy.c
===================================================================
--- modules/proxy/mod_proxy.c	(revision 1729388)
+++ modules/proxy/mod_proxy.c	(working copy)
@@ -280,7 +280,7 @@ static const char *set_worker_param(apr_pool_t *p,
          */
         if (ap_timeout_parameter_parse(val, &timeout, "s") != APR_SUCCESS)
             return "Ping/Pong timeout has wrong format";
-        if (timeout < 1000 && timeout >= 0)
+        if (timeout < 1000)
             return "Ping/Pong timeout must be at least one millisecond";
         worker->s->ping_timeout = timeout;
         worker->s->ping_timeout_set = 1;
Index: modules/proxy/mod_proxy.h
===================================================================
--- modules/proxy/mod_proxy.h	(revision 1729388)
+++ modules/proxy/mod_proxy.h	(working copy)
@@ -365,7 +365,6 @@ do {                             \
 
 #define PROXY_DO_100_CONTINUE(w, r) \
 ((w)->s->ping_timeout_set \
- && ((w)->s->ping_timeout >= 0) \
  && (PROXYREQ_REVERSE == (r)->proxyreq) \
  && !(apr_table_get((r)->subprocess_env, "force-proxy-request-1.0")) \
  && ap_request_has_body((r)))
Index: modules/proxy/mod_proxy_ajp.c
===================================================================
--- modules/proxy/mod_proxy_ajp.c	(revision 1729388)
+++ modules/proxy/mod_proxy_ajp.c	(working copy)
@@ -342,8 +342,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
          * but doesn't affect the whole worker.
          */
         if (APR_STATUS_IS_TIMEUP(status) &&
-            conn->worker->s->ping_timeout_set &&
-            conn->worker->s->ping_timeout >= 0) {
+                conn->worker->s->ping_timeout_set) {
             return HTTP_GATEWAY_TIME_OUT;
         }
 
@@ -680,8 +679,7 @@ static int ap_proxy_ajp_request(apr_pool_t *p, req
              * but doesn't affect the whole worker.
              */
             if (APR_STATUS_IS_TIMEUP(status) &&
-                conn->worker->s->ping_timeout_set &&
-                conn->worker->s->ping_timeout >= 0) {
+                    conn->worker->s->ping_timeout_set) {
                 apr_table_setn(r->notes, "proxy_timedout", "1");
                 rv = HTTP_GATEWAY_TIME_OUT;
             }
@@ -791,36 +789,23 @@ static int proxy_ajp_handler(request_rec *r, proxy
 
         /* Handle CPING/CPONG */
         if (worker->s->ping_timeout_set) {
-            if (worker->s->ping_timeout < 0) {
-                if (!ap_proxy_is_socket_connected(backend->sock)) {
-                    backend->close = 1;
-                    ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(02534)
-                                  "socket check failed to %pI (%s)",
-                                  worker->cp->addr, worker->s->hostname);
-                    status = HTTP_SERVICE_UNAVAILABLE;
-                    retry++;
-                    continue;
-                }
+            status = ajp_handle_cping_cpong(backend->sock, r,
+                                            worker->s->ping_timeout);
+            /*
+             * In case the CPING / CPONG failed for the first time we might be
+             * just out of luck and got a faulty backend connection, but the
+             * backend might be healthy nevertheless. So ensure that the backend
+             * TCP connection gets closed and try it once again.
+             */
+            if (status != APR_SUCCESS) {
+                backend->close = 1;
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
+                              "cping/cpong failed to %pI (%s)",
+                              worker->cp->addr, worker->s->hostname);
+                status = HTTP_SERVICE_UNAVAILABLE;
+                retry++;
+                continue;
             }
-            else {
-                status = ajp_handle_cping_cpong(backend->sock, r,
-                                                worker->s->ping_timeout);
-                /*
-                 * In case the CPING / CPONG failed for the first time we might be
-                 * just out of luck and got a faulty backend connection, but the
-                 * backend might be healthy nevertheless. So ensure that the backend
-                 * TCP connection gets closed and try it once again.
-                 */
-                if (status != APR_SUCCESS) {
-                    backend->close = 1;
-                    ap_log_rerror(APLOG_MARK, APLOG_ERR, status, r, APLOGNO(00897)
-                                  "cping/cpong failed to %pI (%s)",
-                                  worker->cp->addr, worker->s->hostname);
-                    status = HTTP_SERVICE_UNAVAILABLE;
-                    retry++;
-                    continue;
-                }
-            }
         }
         /* Step Three: Process the Request */
         status = ap_proxy_ajp_request(p, r, backend, origin, dconf, uri, locurl,
Index: modules/proxy/mod_proxy_hcheck.c
===================================================================
--- modules/proxy/mod_proxy_hcheck.c	(revision 1729388)
+++ modules/proxy/mod_proxy_hcheck.c	(working copy)
@@ -553,9 +553,6 @@ static apr_status_t hc_check_tcp(sctx_t *ctx, apr_
     if (status == OK) {
         backend->addr = hc->cp->addr;
         status = ap_proxy_connect_backend("HCTCP", backend, hc, ctx->s);
-        if (status == OK) {
-            status = (ap_proxy_is_socket_connected(backend->sock) ? OK : !OK);
-        }
     }
     return backend_cleanup("HCTCP", backend, ctx->s, status);
 }
Index: modules/proxy/mod_proxy_http.c
===================================================================
--- modules/proxy/mod_proxy_http.c	(revision 1729388)
+++ modules/proxy/mod_proxy_http.c	(working copy)
@@ -2097,21 +2092,6 @@ static int proxy_http_handler(request_rec *r, prox
                                "proxy-request-hostname",
                                backend->ssl_hostname);
             }
-
-            /* Step Three-and-a-Half: See if the socket is still connected (if
-             * desired). Note: Since ap_proxy_connect_backend just above does
-             * the same check (unconditionally), this step is not required when
-             * backend's socket/connection is reused (ie. no Step Three).
-             */
-            if (worker->s->ping_timeout_set && worker->s->ping_timeout < 0 &&
-                    !ap_proxy_is_socket_connected(backend->sock)) {
-                backend->close = 1;
-                ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(02535)
-                              "socket check failed to %pI (%s)",
-                              worker->cp->addr, worker->s->hostname);
-                retry++;
-                continue;
-            }
         }
 
         /* Don't recycle the connection if prefetch (above) told not to do so */
@@ -2130,8 +2110,7 @@ static int proxy_http_handler(request_rec *r, prox
                                             flushall)) != OK) {
             proxy_run_detach_backend(r, backend);
             if ((status == HTTP_SERVICE_UNAVAILABLE) &&
-                 worker->s->ping_timeout_set &&
-                 worker->s->ping_timeout >= 0) {
+                    worker->s->ping_timeout_set) {
                 backend->close = 1;
                 ap_log_rerror(APLOG_MARK, APLOG_INFO, status, r, APLOGNO(01115)
                               "HTTP: 100-Continue failed to %pI (%s)",

Reply via email to