From: Joe Orton Date: Thu, 7 Dec 2023 18:25:35 +0000 (+0000) Subject: mod_dav_fs: Add global mutex around use of lockdb use, since X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=455147a36049efc443921ea523d01aa62e047fa3;p=thirdparty%2Fapache%2Fhttpd.git mod_dav_fs: Add global mutex around use of lockdb use, since apr_dbm does not provide thread-safe locking: * modules/dav/fs/mod_dav_fs.c (dav_fs_get_server_conf): Replaces dav_get_lockdb_path. (dav_fs_pre_config, dav_fs_child_init): New hooks. (dav_fs_post_config): Create & store the mutex here. (register_hooks): Register new hooks. * modules/dav/fs/repos.h: Expose new dav_fs_server_conf struct. * modules/dav/fs/lock.c (dav_fs_lockdb_cleanup): New cleanup which unlocks and closes the dbm handle. (dav_fs_really_open_lockdb): Lock the mutex here, register a cleanup. (dav_fs_open_lockdb): Adjust to use dav_fs_get_server_conf. (dav_fs_close_lockdb): Run the cleanup here. Github: closes #395 git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1914438 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/modules/dav/fs/lock.c b/modules/dav/fs/lock.c index c477302ca47..8fc2c126057 100644 --- a/modules/dav/fs/lock.c +++ b/modules/dav/fs/lock.c @@ -181,9 +181,7 @@ struct dav_lockdb_private { request_rec *r; /* for accessing the uuid state */ apr_pool_t *pool; /* a pool to use */ - const char *lockdb_path; /* where is the lock database? */ - const char *lockdb_type; /* where is the lock database? */ - + const dav_fs_server_conf *conf; /* lock database config & metadata */ int opened; /* we opened the database */ dav_db *db; /* if non-NULL, the lock database */ }; @@ -293,6 +291,19 @@ static int dav_fs_compare_locktoken( return dav_compare_locktoken(lt1, lt2); } +static apr_status_t dav_fs_lockdb_cleanup(void *data) +{ + dav_lockdb *lockdb = data; + + apr_global_mutex_unlock(lockdb->info->conf->lockdb_mutex); + + if (lockdb->info->db) { + dav_dbm_close(lockdb->info->db); + } + + return APR_SUCCESS; +} + /* ** dav_fs_really_open_lockdb: ** @@ -301,16 +312,27 @@ static int dav_fs_compare_locktoken( static dav_error * dav_fs_really_open_lockdb(dav_lockdb *lockdb) { dav_error *err; + apr_status_t rv; if (lockdb->info->opened) return NULL; + rv = apr_global_mutex_lock(lockdb->info->conf->lockdb_mutex); + if (rv) { + return dav_new_error(lockdb->info->pool, + HTTP_INTERNAL_SERVER_ERROR, + DAV_ERR_LOCK_OPENDB, rv, + "Could not lock mutex for lock database."); + } + err = dav_dbm_open_direct(lockdb->info->pool, - lockdb->info->lockdb_path, - lockdb->info->lockdb_type, + lockdb->info->conf->lockdb_path, + lockdb->info->conf->lockdb_type, lockdb->ro, &lockdb->info->db); if (err != NULL) { + apr_global_mutex_unlock(lockdb->info->conf->lockdb_mutex); + return dav_push_error(lockdb->info->pool, HTTP_INTERNAL_SERVER_ERROR, DAV_ERR_LOCK_OPENDB, @@ -318,6 +340,10 @@ static dav_error * dav_fs_really_open_lockdb(dav_lockdb *lockdb) err); } + apr_pool_cleanup_register(lockdb->info->pool, lockdb, + dav_fs_lockdb_cleanup, + dav_fs_lockdb_cleanup); + /* all right. it is opened now. */ lockdb->info->opened = 1; @@ -343,9 +369,9 @@ static dav_error * dav_fs_open_lockdb(request_rec *r, int ro, int force, comb->pub.info = &comb->priv; comb->priv.r = r; comb->priv.pool = r->pool; - - comb->priv.lockdb_path = dav_get_lockdb_path(r, &comb->priv.lockdb_type); - if (comb->priv.lockdb_path == NULL) { + comb->priv.conf = dav_fs_get_server_conf(r); + + if (comb->priv.conf == NULL || comb->priv.conf->lockdb_path == NULL) { return dav_new_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, DAV_ERR_LOCK_NO_DB, 0, "A lock database was not specified with the " @@ -371,8 +397,8 @@ static dav_error * dav_fs_open_lockdb(request_rec *r, int ro, int force, */ static void dav_fs_close_lockdb(dav_lockdb *lockdb) { - if (lockdb->info->db != NULL) - dav_dbm_close(lockdb->info->db); + apr_pool_cleanup_run(lockdb->info->pool, lockdb, + dav_fs_lockdb_cleanup); } /* diff --git a/modules/dav/fs/mod_dav_fs.c b/modules/dav/fs/mod_dav_fs.c index f76d21c8d06..5d73f9bfc8c 100644 --- a/modules/dav/fs/mod_dav_fs.c +++ b/modules/dav/fs/mod_dav_fs.c @@ -14,13 +14,16 @@ * limitations under the License. */ +#if !defined(_MSC_VER) && !defined(NETWARE) +#include "ap_config_auto.h" +#endif + #include "httpd.h" #include "http_config.h" +#include "http_core.h" +#include "http_log.h" #include "http_request.h" #include "apr_strings.h" -#if !defined(_MSC_VER) && !defined(NETWARE) -#include "ap_config_auto.h" -#endif #include "mod_dav.h" #include "repos.h" @@ -31,12 +34,6 @@ typedef struct { apr_off_t quota; } dav_fs_dir_conf; -/* per-server configuration */ -typedef struct { - const char *lockdb_path; - const char *lockdb_type; -} dav_fs_server_conf; - extern module AP_MODULE_DECLARE_DATA dav_fs_module; #ifndef DEFAULT_DAV_LOCKDB @@ -46,16 +43,13 @@ extern module AP_MODULE_DECLARE_DATA dav_fs_module; #define DEFAULT_DAV_LOCKDB_TYPE "default" #endif +static const char dav_fs_mutexid[] = "dav_fs-lockdb"; -const char *dav_get_lockdb_path(const request_rec *r, const char **dbmtype) -{ - dav_fs_server_conf *conf; - - conf = ap_get_module_config(r->server->module_config, &dav_fs_module); +static apr_global_mutex_t *dav_fs_lockdb_mutex; - *dbmtype = conf->lockdb_type; - - return conf->lockdb_path; +const dav_fs_server_conf *dav_fs_get_server_conf(const request_rec *r) +{ + return ap_get_module_config(r->server->module_config, &dav_fs_module); } dav_error *dav_fs_get_quota(const request_rec *r, const char *path, @@ -162,12 +156,44 @@ static void *dav_fs_merge_dir_config(apr_pool_t *p, void *base, void *overrides) return newconf; } +static int dav_fs_pre_config(apr_pool_t *pconf, apr_pool_t *plog, apr_pool_t *ptemp) +{ + if (ap_mutex_register(pconf, dav_fs_mutexid, NULL, APR_LOCK_DEFAULT, 0)) + return !OK; + return OK; +} + +static void dav_fs_child_init(apr_pool_t *p, server_rec *s) +{ + apr_status_t rv; + + rv = apr_global_mutex_child_init(&dav_fs_lockdb_mutex, + apr_global_mutex_lockfile(dav_fs_lockdb_mutex), + p); + if (rv) { + ap_log_error(APLOG_MARK, APLOG_ERR, rv, s, + APLOGNO(10488) "child init failed for mutex"); + } +} static apr_status_t dav_fs_post_config(apr_pool_t *p, apr_pool_t *plog, apr_pool_t *ptemp, server_rec *base_server) { server_rec *s; - + apr_status_t rv; + + /* Ignore first pass through the config. */ + if (ap_state_query(AP_SQ_MAIN_STATE) == AP_SQ_MS_CREATE_PRE_CONFIG) + return OK; + + rv = ap_global_mutex_create(&dav_fs_lockdb_mutex, NULL, dav_fs_mutexid, NULL, + base_server, p, 0); + if (rv) { + ap_log_error(APLOG_MARK, APLOG_ERR, rv, base_server, + APLOGNO(10489) "could not create lock mutex"); + return !OK; + } + for (s = base_server; s; s = s->next) { dav_fs_server_conf *conf; @@ -179,6 +205,10 @@ static apr_status_t dav_fs_post_config(apr_pool_t *p, apr_pool_t *plog, if (!conf->lockdb_type) { conf->lockdb_type = DEFAULT_DAV_LOCKDB_TYPE; } + + /* Mutex is common across all vhosts, but could have one per + * vhost if required. */ + conf->lockdb_mutex = dav_fs_lockdb_mutex; } return OK; @@ -252,8 +282,10 @@ static const command_rec dav_fs_cmds[] = static void register_hooks(apr_pool_t *p) { + ap_hook_pre_config(dav_fs_pre_config, NULL, NULL, APR_HOOK_MIDDLE); ap_hook_post_config(dav_fs_post_config, NULL, NULL, APR_HOOK_MIDDLE); - + ap_hook_child_init(dav_fs_child_init, NULL, NULL, APR_HOOK_MIDDLE); + dav_hook_gather_propsets(dav_fs_gather_propsets, NULL, NULL, APR_HOOK_MIDDLE); dav_hook_find_liveprop(dav_fs_find_liveprop, NULL, NULL, APR_HOOK_MIDDLE); diff --git a/modules/dav/fs/repos.h b/modules/dav/fs/repos.h index 849ba5f21f7..733383ed91d 100644 --- a/modules/dav/fs/repos.h +++ b/modules/dav/fs/repos.h @@ -25,6 +25,8 @@ #ifndef _DAV_FS_REPOS_H_ #define _DAV_FS_REPOS_H_ +#include "util_mutex.h" + /* the subdirectory to hold all DAV-related information for a directory */ #define DAV_FS_STATE_DIR ".DAV" #define DAV_FS_STATE_FILE_FOR_DIR ".state_for_dir" @@ -77,9 +79,15 @@ void dav_dbm_freedatum(dav_db *db, apr_datum_t data); int dav_dbm_exists(dav_db *db, apr_datum_t key); void dav_dbm_close(dav_db *db); -/* Returns path to lock database and configured dbm type as - * *dbmtype. */ -const char *dav_get_lockdb_path(const request_rec *r, const char **dbmtype); +/* Per-server configuration. */ +typedef struct { + const char *lockdb_path; + const char *lockdb_type; + apr_global_mutex_t *lockdb_mutex; +} dav_fs_server_conf; + +/* Returns server configuration for the request. */ +const dav_fs_server_conf *dav_fs_get_server_conf(const request_rec *r); dav_error *dav_fs_get_quota(const request_rec *r, const char *path, apr_off_t *quota_bytes);