Skip to content

Commit

Permalink
mod_dav_fs: Add global mutex around use of lockdb use, since
Browse files Browse the repository at this point in the history 
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 apache#395


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1914438 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
@notroj
notroj committed Dec 7, 2023
1 parent bd3bc70 commit 455147a
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 32 deletions.
46 changes: 36 additions & 10 deletions modules/dav/fs/lock.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
};
Expand Down Expand Up @@ -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:
**
Expand All @@ -301,23 +312,38 @@ 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,
"Could not open the lock database.",
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;

Expand All @@ -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 "
Expand All @@ -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);
}

/*
Expand Down
70 changes: 51 additions & 19 deletions modules/dav/fs/mod_dav_fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 11 additions & 3 deletions modules/dav/fs/repos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 455147a

Please sign in to comment.