From: Ruediger Pluem <rpluem@apache.org>
Date: Fri, 14 Apr 2006 13:20:28 +0000 (+0000)
Subject: * Avoid calling ap_proxy_http_cleanup twice as this releases a connection
X-Git-Tag: 2.3.0~2453
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96c6356987991704cbf490733c21d4d605f27b9a;p=thirdparty%2Fapache%2Fhttpd.git

* Avoid calling ap_proxy_http_cleanup twice as this releases a connection
  from the connection pool twice. This causes this connection to be present
  in the connection pool twice. Thus it may be used by different threads
  at the same time which causes many troubles (segfaults in this case).
  Furthermore implement a logic to prevent double releases to the connection
  pool if they are triggered by buggy code and log an error message in this
  case.

  - mod_proxy_http.c: remove double calls to ap_proxy_http_cleanup
  - proxy_util.c: Add logic to prevent double releases of a
    connection to the connection pool.

PR: 38793


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

diff --git a/CHANGES b/CHANGES
index ddaed69d495..525ec7b5ff1 100644
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 Changes with Apache 2.3.0
   [Remove entries to the current 2.0 and 2.2 section below, when backported]
 
+  *) mod_proxy: Do not release connections from connection pool twice.
+     PR 38793. [Ruediger Pluem, matthias <mk-asf gigacodes.de>]
+
   *) core: Prevent reading uninitialized memory while reading a line of
      protocol input.  PR 39282. [Davi Arnaut <davi haxent.com.br>]
 
diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 2590a9c9f78..09bb1cd28aa 100644
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -119,6 +119,7 @@
  *                         cache_server_conf (minor)
  * 20060110.2 (2.3.0-dev)  flush_packets and flush_wait members added to
  *                         proxy_server (minor)
+ * 20060110.3 (2.3.0-dev)  added inreslist member to proxy_conn_rec (minor)
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */
@@ -126,7 +127,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20060110
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 2                     /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 3                     /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h
index 11ce04aff09..72ec3739e88 100644
--- a/modules/proxy/mod_proxy.h
+++ b/modules/proxy/mod_proxy.h
@@ -221,6 +221,9 @@ typedef struct {
     int          close_on_recycle; /* Close the connection when returning to pool */
     proxy_worker *worker;   /* Connection pool this connection belogns to */
     void         *data;     /* per scheme connection data */
+#if APR_HAS_THREADS
+    int          inreslist; /* connection in apr_reslist? */
+#endif
 } proxy_conn_rec;
 
 typedef struct {
diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 9d439a61ee9..8dd5ca0bf72 100644
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -1230,7 +1230,6 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
                           "proxy: error reading status line from remote "
                           "server %s", backend->hostname);
-            ap_proxy_http_cleanup(NULL, r, backend);
             return ap_proxyerror(r, HTTP_BAD_GATEWAY,
                                  "Error reading from remote server");
         }
@@ -1251,7 +1250,6 @@ apr_status_t ap_proxy_http_process_response(apr_pool_t * p, request_rec *r,
              * if the status line was > 8192 bytes
              */
             else if ((buffer[5] != '1') || (len >= sizeof(buffer)-1)) {
-                ap_proxy_http_cleanup(NULL, r, backend);
                 return ap_proxyerror(r, HTTP_BAD_GATEWAY,
                 apr_pstrcat(p, "Corrupt status line returned by remote "
                             "server: ", buffer, NULL));
diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 104b465e685..a63bfcaf636 100644
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -1512,7 +1512,18 @@ static apr_status_t connection_cleanup(void *theconn)
     if (!worker->cp)
         return APR_SUCCESS;
 
-    /* deterimine if the connection need to be closed */
+#if APR_HAS_THREADS
+    /* Sanity check: Did we already return the pooled connection? */
+    if (conn->inreslist) {
+        ap_log_perror(APLOG_MARK, APLOG_ERR, 0, conn->pool,
+                      "proxy: Pooled connection 0x%pp for worker %s has been"
+                      " already returned to the connection pool.", conn,
+                      worker->name);
+        return APR_SUCCESS;
+    }
+#endif
+
+    /* determine if the connection need to be closed */
     if (conn->close_on_recycle || conn->close) {
         apr_pool_t *p = conn->pool;
         apr_pool_clear(conn->pool);
@@ -1522,6 +1533,7 @@ static apr_status_t connection_cleanup(void *theconn)
     }
 #if APR_HAS_THREADS
     if (worker->hmax && worker->cp->res) {
+        conn->inreslist = 1;
         apr_reslist_release(worker->cp->res, (void *)conn);
     }
     else
@@ -1552,6 +1564,7 @@ static apr_status_t connection_constructor(void **resource, void *params,
 
     conn->pool   = ctx;
     conn->worker = worker;
+    conn->inreslist = 1;
     *resource = conn;
 
     return APR_SUCCESS;
@@ -1784,6 +1797,7 @@ PROXY_DECLARE(int) ap_proxy_acquire_connection(const char *proxy_function,
     (*conn)->worker = worker;
     (*conn)->close  = 0;
     (*conn)->close_on_recycle = 0;
+    (*conn)->inreslist = 0;
 
     return OK;
 }