2.4 mod_dav_fs: add DAVLockDBType and add global mutex around lockdb use

changes-entries/davlockdbtype.txt

@@ -0,0 +1 @@
+  *) mod_dav_fs: Add DavLockDBType directive.  [Joe Orton]

docs/manual/mod/mod_dav_fs.xml

@@ -58,11 +58,12 @@
 
 <usage>
     <p>Use the <directive>DavLockDB</directive> directive to specify
-    the full path to the lock database, excluding an extension. If
-    the path is not absolute, it will be taken relative to <directive
+    the full path to the lock database, excluding an extension. If the
+    path is not absolute, it will be taken relative to <directive
     module="core">ServerRoot</directive>. The implementation of
-    <module>mod_dav_fs</module> uses a SDBM database to track user
-    locks.</p>
+    <module>mod_dav_fs</module> uses a database to track user
+    locks. The type of database file can be changed using
+    <directive module="mod_dav_fs">DavLockDBType</directive>.</p>
 
     <!-- XXX: someone should add that tool to the support directory...
     The utility <code>modules/dav/util/lockview</code> can be used from
@@ -87,6 +88,23 @@
 </usage>
 </directivesynopsis>
 
+<directivesynopsis>
+<name>DavLockDBType</name>
+<description>Database type of the DAV lock database</description>
+<syntax>DavLockDBType <var>dbm-type</var></syntax>
+<contextlist><context>server config</context><context>virtual host</context>
+</contextlist>
+<default>DavLockDBType default</default>
+<compatibility>Available in version 2.5.1 and later.</compatibility>
+<usage>
+    <p>Use the <directive>DavLockDBType</directive> directive to
+    specify the type of the database file. If the directive is not
+    used, the <code>apr_dbm</code> hard-coded default database type is
+    used, which is equivalent to using <code>DavLockDBType default</code>.</p>
+
+</usage>
+</directivesynopsis>
+
 <directivesynopsis>
 <name>DavLockDiscovery</name>
 <description>Enable lock discovery</description>

modules/dav/fs/dbm.c

@@ -47,6 +47,10 @@
 #include "http_log.h"
 #include "http_main.h"      /* for ap_server_conf */
 
+#ifndef DEFAULT_PROPDB_DBM_TYPE
+#define DEFAULT_PROPDB_DBM_TYPE "default"
+#endif
+
 APLOG_USE_MODULE(dav_fs);
 
 struct dav_db {
@@ -100,7 +104,7 @@ static dav_error * dav_fs_dbm_error(dav_db *db, apr_pool_t *p,
     /* There might not be a <db> if we had problems creating it. */
     if (db == NULL) {
         errcode = 1;
-        errstr = "Could not open property database.";
+        errstr = "Could not open database.";
         if (APR_STATUS_IS_EDSOOPEN(status))
             ap_log_error(APLOG_MARK, APLOG_CRIT, status, ap_server_conf, APLOGNO(00576)
             "The DBM driver could not be loaded");
@@ -129,10 +133,10 @@ void dav_fs_ensure_state_dir(apr_pool_t * p, const char *dirname)
 /* dav_dbm_open_direct:  Opens a *dbm database specified by path.
  *    ro = boolean read-only flag.
  */
-dav_error * dav_dbm_open_direct(apr_pool_t *p, const char *pathname, int ro,
-                                dav_db **pdb)
+dav_error * dav_dbm_open_direct(apr_pool_t *p, const char *pathname,
+                                const char *dbmtype, int ro, dav_db **pdb)
 {
-#if APU_MAJOR_VERSION > 1 || (APU_MAJOR_VERSION == 1 && APU_MINOR_VERSION >= 7)
+#if APR_MAJOR_VERSION > 1 || (APU_MAJOR_VERSION == 1 && APU_MINOR_VERSION >= 7)
     const apr_dbm_driver_t *driver;
     const apu_err_t *err;
 #endif
@@ -141,13 +145,13 @@ dav_error * dav_dbm_open_direct(apr_pool_t *p, const char *pathname, int ro,
 
     *pdb = NULL;
 
-#if APU_MAJOR_VERSION > 1 || (APU_MAJOR_VERSION == 1 && APU_MINOR_VERSION >= 7)
-    if ((status = apr_dbm_get_driver(&driver, NULL, &err, p)) != APR_SUCCESS) {
+#if APR_MAJOR_VERSION > 1 || (APU_MAJOR_VERSION == 1 && APU_MINOR_VERSION >= 7)
+    if ((status = apr_dbm_get_driver(&driver, dbmtype, &err, p)) != APR_SUCCESS) {
         ap_log_error(APLOG_MARK, APLOG_ERR, status, ap_server_conf, APLOGNO(10289)
-                     "mod_dav_fs: The DBM library '%s' could not be loaded: %s",
-                             err->reason, err->msg);
+                     "mod_dav_fs: The DBM library '%s' for '%s' could not be loaded: %s",
+                     err->reason, dbmtype, err->msg);
         return dav_new_error(p, HTTP_INTERNAL_SERVER_ERROR, 1, status,
-                "Could not load library for property database.");
+                "Could not load library for database.");
     }
     if ((status = apr_dbm_open2(&file, driver, pathname,
                                ro ? APR_DBM_READONLY : APR_DBM_RWCREATE,
@@ -156,9 +160,9 @@ dav_error * dav_dbm_open_direct(apr_pool_t *p, const char *pathname, int ro,
         return dav_fs_dbm_error(NULL, p, status);
     }
 #else
-    if ((status = apr_dbm_open(&file, pathname,
-                               ro ? APR_DBM_READONLY : APR_DBM_RWCREATE,
-                               APR_OS_DEFAULT, p))
+    if ((status = apr_dbm_open_ex(&file, dbmtype, pathname,
+                                  ro ? APR_DBM_READONLY : APR_DBM_RWCREATE,
+                                  APR_OS_DEFAULT, p))
                 != APR_SUCCESS
         && !ro) {
         /* ### do something with 'status' */
@@ -206,7 +210,7 @@ static dav_error * dav_dbm_open(apr_pool_t * p, const dav_resource *resource,
 
     /* ### do we need to deal with the umask? */
 
-    return dav_dbm_open_direct(p, pathname, ro, pdb);
+    return dav_dbm_open_direct(p, pathname, DEFAULT_PROPDB_DBM_TYPE, ro, pdb);
 }
 
 void dav_dbm_close(dav_db *db)

modules/dav/fs/lock.c

@@ -181,8 +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 dav_fs_server_conf *conf;  /* lock database config & metadata */
     int opened;                      /* we opened the database */
     dav_db *db;                      /* if non-NULL, the lock database */
 };
@@ -292,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:
 **
@@ -300,22 +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->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;
 
@@ -341,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);
-    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 "
@@ -369,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);
 }
 
 /*

modules/dav/fs/mod_dav_fs.c

@@ -14,27 +14,36 @@
  * 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"
 
 #include "mod_dav.h"
 #include "repos.h"
 
-/* per-server configuration */
-typedef struct {
-    const char *lockdb_path;
+extern module AP_MODULE_DECLARE_DATA dav_fs_module;
 
-} dav_fs_server_conf;
+#ifndef DEFAULT_DAV_LOCKDB
+#define DEFAULT_DAV_LOCKDB "davlockdb"
+#endif
+#ifndef DEFAULT_DAV_LOCKDB_TYPE
+#define DEFAULT_DAV_LOCKDB_TYPE "default"
+#endif
 
-extern module AP_MODULE_DECLARE_DATA dav_fs_module;
+static const char dav_fs_mutexid[] = "dav_fs-lockdb";
 
-const char *dav_get_lockdb_path(const request_rec *r)
-{
-    dav_fs_server_conf *conf;
+static apr_global_mutex_t *dav_fs_lockdb_mutex;
 
-    conf = ap_get_module_config(r->server->module_config, &dav_fs_module);
-    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);
 }
 
 static void *dav_fs_create_server_config(apr_pool_t *p, server_rec *s)
@@ -53,10 +62,74 @@ static void *dav_fs_merge_server_config(apr_pool_t *p,
 
     newconf->lockdb_path =
         child->lockdb_path ? child->lockdb_path : parent->lockdb_path;
+    newconf->lockdb_type =
+        child->lockdb_type ? child->lockdb_type : parent->lockdb_type;
 
     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;
+
+        conf = ap_get_module_config(s->module_config, &dav_fs_module);
+
+        if (!conf->lockdb_path) {
+#if AP_MODULE_MAGIC_AT_LEAST(20180906, 2)
+            conf->lockdb_path = ap_state_dir_relative(p, DEFAULT_DAV_LOCKDB);
+#else
+            conf->lockdb_path = ap_runtime_dir_relative(p, DEFAULT_DAV_LOCKDB);
+#endif
+        }
+        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;
+}
+
 /*
  * Command handler for the DAVLockDB directive, which is TAKE1
  */
@@ -76,17 +149,37 @@ static const char *dav_fs_cmd_davlockdb(cmd_parms *cmd, void *config,
     return NULL;
 }
 
+/*
+ * Command handler for the DAVLockDBType directive, which is TAKE1
+ */
+static const char *dav_fs_cmd_davlockdbtype(cmd_parms *cmd, void *config,
+                                        const char *arg1)
+{
+    dav_fs_server_conf *conf = ap_get_module_config(cmd->server->module_config,
+                                                    &dav_fs_module);
+    conf->lockdb_type = arg1;
+
+    return NULL;
+}
+
 static const command_rec dav_fs_cmds[] =
 {
     /* per server */
     AP_INIT_TAKE1("DAVLockDB", dav_fs_cmd_davlockdb, NULL, RSRC_CONF,
                   "specify a lock database"),
+    AP_INIT_TAKE1("DAVLockDBType", dav_fs_cmd_davlockdbtype, NULL, RSRC_CONF,
+                  "specify a lock database DBM type"),
 
     { NULL }
 };
 
 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);