File httpd-2.2.x.doublefree.patch of Package apache2
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);
