apache2/httpd-2.2.x.doublefree.patch

Attempt of a backport of 
http://svn.apache.org/viewvc?diff_format=h&view=rev&revision=496831

See 
http://issues.apache.org/bugzilla/show_bug.cgi?id=39985

poeml@suse.de

Index: modules/database/mod_dbd.c
===================================================================
--- modules/database/mod_dbd.c	(revision 497960)
+++ modules/database/mod_dbd.c	(working copy)
@@ -50,10 +50,11 @@
     const char *params;
     int persist;
     dbd_prepared *prepared;
+    apr_pool_t *pool;
 #if APR_HAS_THREADS
     apr_thread_mutex_t *mutex;
-    apr_pool_t *pool;
     apr_reslist_t *dbpool;
+    int destroyed;
     int nmin;
     int nkeep;
     int nmax;
@@ -241,6 +242,12 @@
     }
     return ret;
 }
+static apr_status_t dbd_close(void *data)
+{
+    ap_dbd_t *rec = data;
+    return apr_dbd_close(rec->driver, rec->handle);
+}
+
 /************ svr cfg: manage db connection pool ****************/
 /* an apr_reslist_constructor for SQL connections
  * Also use this for opening in non-reslist modes, since it gives
@@ -249,16 +256,20 @@
 static apr_status_t dbd_construct(void **db, void *params, apr_pool_t *pool)
 {
     svr_cfg *svr = (svr_cfg*) params;
-    ap_dbd_t *rec = apr_pcalloc(pool, sizeof(ap_dbd_t));
+    apr_pool_t *rec_pool, *prepared_pool;
+    ap_dbd_t *rec;
     apr_status_t rv;
 
-    /* this pool is mostly so dbd_close can destroy the prepared stmts */
-    rv = apr_pool_create(&rec->pool, pool);
+    rv = apr_pool_create(&rec_pool, pool);
     if (rv != APR_SUCCESS) {
         ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
                       "DBD: Failed to create memory pool");
     }
 
+    rec = apr_pcalloc(rec_pool, sizeof(ap_dbd_t));
+
+    rec->pool = rec_pool;
+
 /* The driver is loaded at config time now, so this just checks a hash.
  * If that changes, the driver DSO could be registered to unload against
  * our pool, which is probably not what we want.  Error checking isn't
@@ -299,63 +310,94 @@
     case APR_SUCCESS:
         break;
     }
-    *db = rec;
-    rv = dbd_prepared_init(rec->pool, svr, rec);
+
+    apr_pool_cleanup_register(rec->pool, rec, dbd_close,
+                              apr_pool_cleanup_null);
+
+    /* we use a sub-pool for the prepared statements for each connection so
+     * that they will be cleaned up first, before the connection is closed
+     */
+    rv = apr_pool_create(&prepared_pool, rec->pool);
     if (rv != APR_SUCCESS) {
+        ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, rec->pool,
+                     "DBD: Failed to create memory pool");
+
+        apr_pool_destroy(rec->pool);
+        return rv;
+    }
+
+    rv = dbd_prepared_init(prepared_pool, svr, rec);
+    if (rv != APR_SUCCESS) {
         const char *errmsg = apr_dbd_error(rec->driver, rec->handle, rv);
         ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, rec->pool,
                       "DBD: failed to initialise prepared SQL statements: %s",
                       (errmsg ? errmsg : "[???]"));
     }
+
+    *db = rec;
+
     return rv;
 }
-static apr_status_t dbd_close(void *CONN)
-{
-    ap_dbd_t *conn = CONN;
-    apr_status_t rv = apr_dbd_close(conn->driver, conn->handle);
-    apr_pool_destroy(conn->pool);
-    return rv;
-}
 #if APR_HAS_THREADS
 static apr_status_t dbd_destruct(void *sql, void *params, apr_pool_t *pool)
 {
-    return dbd_close(sql);
+    svr_cfg *svr = params;
+
+    if (!svr->destroyed) {
+        ap_dbd_t *rec = sql;
+
+        apr_pool_destroy(rec->pool);
+    }
+
+    return APR_SUCCESS;
 }
 
-static apr_status_t dbd_setup(apr_pool_t *pool, svr_cfg *svr)
+static apr_status_t dbd_destroy(void *data)
 {
+    svr_cfg *svr = data;
+
+    svr->destroyed = 1;
+
+    return APR_SUCCESS;
+}
+
+static apr_status_t dbd_setup(server_rec *s, svr_cfg *svr)
+{
     apr_status_t rv;
 
-    /* create a pool just for the reslist from a process-lifetime pool;
-     * that pool (s->process->pool in the dbd_setup_lock case,
-     * whatever was passed to ap_run_child_init in the dbd_setup_init case)
-     * will be shared with other threads doing other non-mod_dbd things
-     * so we can't use it for the reslist directly
+    /* We create the reslist using a sub-pool of the pool passed to our
+     * child_init hook.  No other threads can be here because we're
+     * either in the child_init phase or dbd_setup_lock() acquired our mutex.
+     * No other threads will use this sub-pool after this, except via
+     * reslist calls, which have an internal mutex.
+     *
+     * We need to short-circuit the cleanup registered internally by
+     * apr_reslist_create().  We do this by registering dbd_destroy()
+     * as a cleanup afterwards, so that it will run before the reslist's
+     * internal cleanup.
+     *
+     * If we didn't do this, then we could free memory twice when the pool
+     * was destroyed.  When apr_pool_destroy() runs, it first destroys all
+     * all the per-connection sub-pools created in dbd_construct(), and
+     * then it runs the reslist's cleanup.  The cleanup calls dbd_destruct()
+     * on each resource, which would then attempt to destroy the sub-pools
+     * a second time.
      */
-    rv = apr_pool_create(&svr->pool, pool);
-    if (rv != APR_SUCCESS) {
-        ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
-                      "DBD: Failed to create reslist memory pool");
-        return rv;
-    }
-
     rv = apr_reslist_create(&svr->dbpool, svr->nmin, svr->nkeep, svr->nmax,
                             apr_time_from_sec(svr->exptime),
                             dbd_construct, dbd_destruct, svr, svr->pool);
-    if (rv == APR_SUCCESS) {
-        apr_pool_cleanup_register(svr->pool, svr->dbpool,
-                                  (void*)apr_reslist_destroy,
-                                  apr_pool_cleanup_null);
-    }
-    else {
-        ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, svr->pool,
+    if (rv != APR_SUCCESS) {
+        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
                       "DBD: failed to initialise");
-        apr_pool_destroy(svr->pool);
-        svr->pool = NULL;
+	return rv;
     }
 
-    return rv;
+    apr_pool_cleanup_register(svr->pool, svr, dbd_destroy,
+                              apr_pool_cleanup_null);
+    return APR_SUCCESS;
 }
+#endif
+
 static apr_status_t dbd_setup_init(apr_pool_t *pool, server_rec *s)
 {
     svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module);
@@ -374,7 +416,15 @@
         return APR_SUCCESS;
     }
 
-    rv = dbd_setup(pool, svr);
+    rv = apr_pool_create(&svr->pool, pool);
+    if (rv != APR_SUCCESS) {
+        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
+                     "DBD: Failed to create reslist cleanup memory pool");
+        return rv;
+    }
+
+#if APR_HAS_THREADS
+    rv = dbd_setup(s, svr);
     if (rv == APR_SUCCESS) {
         return rv;
     }
@@ -387,9 +437,12 @@
         ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
                       "DBD: Failed to create thread mutex");
     }
+#endif
+
     return rv;
 }
-static apr_status_t dbd_setup_lock(apr_pool_t *pool, server_rec *s)
+#if APR_HAS_THREADS
+static apr_status_t dbd_setup_lock(server_rec *s)
 {
     svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module);
     apr_status_t rv, rv2 = APR_SUCCESS;
@@ -404,18 +457,18 @@
 
     rv = apr_thread_mutex_lock(svr->mutex);
     if (rv != APR_SUCCESS) {
-        ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
+        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
                       "DBD: Failed to acquire thread mutex");
         return rv;
     }
 
     if (!svr->dbpool) {
-        rv2 = dbd_setup(s->process->pool, svr);
+        rv2 = dbd_setup(s, svr);
     }
 
     rv = apr_thread_mutex_unlock(svr->mutex);
     if (rv != APR_SUCCESS) {
-        ap_log_perror(APLOG_MARK, APLOG_CRIT, rv, pool,
+        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s,
                       "DBD: Failed to release thread mutex");
         if (rv2 == APR_SUCCESS) {
             rv2 = rv;
@@ -434,7 +487,7 @@
 {
     svr_cfg *svr = ap_get_module_config(s->module_config, &dbd_module);
     if (!svr->persist) {
-        dbd_close((void*) sql);
+        apr_pool_destroy(sql->pool);
     }
 #if APR_HAS_THREADS
     else {
@@ -459,12 +512,12 @@
 
     if (!svr->persist) {
         /* Return a once-only connection */
-        rv = dbd_construct(&rec, svr, s->process->pool);
+        dbd_construct((void*) &rec, svr, pool);
         return (rv == APR_SUCCESS) ? arec : NULL;
     }
 
     if (!svr->dbpool) {
-        if (dbd_setup_lock(pool, s) != APR_SUCCESS) {
+        if (dbd_setup_lock(s) != APR_SUCCESS) {
             return NULL;
         }
     }
@@ -503,7 +556,7 @@
 
     if (!svr->persist) {
         /* Return a once-only connection */
-        rv = dbd_construct(&rec, svr, s->process->pool);
+        dbd_construct((void*) &rec, svr, pool);
         return (rv == APR_SUCCESS) ? arec : NULL;
     }
 
@@ -519,14 +572,14 @@
             ap_log_perror(APLOG_MARK, APLOG_ERR, rv, pool,
                           "DBD[%s] Error: %s", svr->name, errmsg);
             svr->conn = NULL;
+            apr_pool_destroy(rec->pool);
         }
     }
+
 /* We don't have a connection right now, so we'll open one */
     if (!svr->conn) {
-        if (dbd_construct(&rec, svr, s->process->pool) == APR_SUCCESS) {
+        if (dbd_construct(&rec, svr, svr->pool) == APR_SUCCESS) {
             svr->conn = arec ;
-            apr_pool_cleanup_register(s->process->pool, svr->conn,
-                                      dbd_close, apr_pool_cleanup_null);
         }
     }
     return svr->conn;
@@ -569,10 +622,6 @@
                 apr_pool_cleanup_register(r->pool, req, dbd_release,
                                           apr_pool_cleanup_null);
             }
-            else {
-                apr_pool_cleanup_register(r->pool, req->conn, dbd_close,
-                                          apr_pool_cleanup_null);
-            }
         }
     }
     return req->conn;
@@ -592,10 +641,6 @@
                 apr_pool_cleanup_register(c->pool, req, dbd_release,
                                           apr_pool_cleanup_null);
             }
-            else {
-                apr_pool_cleanup_register(c->pool, req->conn, dbd_close,
-                                          apr_pool_cleanup_null);
-            }
         }
     }
     return req->conn;
@@ -617,15 +662,9 @@
 
     ret = ap_get_module_config(r->request_config, &dbd_module);
     if (!ret) {
-        svr = ap_get_module_config(r->server->module_config, &dbd_module);
         ret = ap_dbd_open(r->pool, r->server);
         if (ret) {
             ap_set_module_config(r->request_config, &dbd_module, ret);
-            if (!svr->persist) {
-                apr_pool_cleanup_register(r->pool, svr->conn, dbd_close,
-                                          apr_pool_cleanup_null);
-            }
-            /* if persist then dbd_open registered cleanup on proc pool */
         }
     }
     return ret;
@@ -635,15 +674,9 @@
     svr_cfg *svr;
     ap_dbd_t *ret = ap_get_module_config(c->conn_config, &dbd_module);
     if (!ret) {
-        svr = ap_get_module_config(c->base_server->module_config, &dbd_module);
         ret = ap_dbd_open(c->pool, c->base_server);
         if (ret) {
             ap_set_module_config(c->conn_config, &dbd_module, ret);
-            if (!svr->persist) {
-                apr_pool_cleanup_register(c->pool, svr->conn, dbd_close,
-                                          apr_pool_cleanup_null);
-            }
-            /* if persist then dbd_open registered cleanup on proc pool */
         }
     }
     return ret;
@@ -670,9 +703,7 @@
 }
 static void dbd_hooks(apr_pool_t *pool)
 {
-#if APR_HAS_THREADS
     ap_hook_child_init((void*)dbd_setup_init, NULL, NULL, APR_HOOK_MIDDLE);
-#endif
     APR_REGISTER_OPTIONAL_FN(ap_dbd_open);
     APR_REGISTER_OPTIONAL_FN(ap_dbd_close);
     APR_REGISTER_OPTIONAL_FN(ap_dbd_acquire);