* 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 ddaed69d49535e283504190c975ef0f1bf92b9e4..525ec7b5ff10f5a6e374e143fa204e45117bbb77 100644 (file)
--- 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 2590a9c9f78212ad8db2d371a5904dbd02ceb186..09bb1cd28aa4d9a3f83b3fe2d5a26e99d32ea996 100644 (file)
--- 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 11ce04aff09e9ce38d7bfbff19b6408918b93686..72ec3739e8821e20a543ded5792f8efb27756929 100644 (file)
--- 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 9d439a61ee9548d2f7f7cda0916e4d137923b924..8dd5ca0bf72c9866a8aa16f56b8c07e35ca033e0 100644 (file)
--- 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 104b465e68563c8321c51b9873cce6da946eca92..a63bfcaf636a21fd0c51d02b6d161f78ae92e7c1 100644 (file)
--- 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;
 }