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 c477302ca473a47a031494b06ae448c27931dce1..8fc2c12605731a4034ec76ea25ae765267d1236a 100644 (file)
--- 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 f76d21c8d0625853dc044e7c4dd26cbf3d06e920..5d73f9bfc8c5aaef5c664c2b6c818494b54b0d7a 100644 (file)
--- 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 849ba5f21f7af2d1d5299e2b5b061f016b4051cd..733383ed91d2ce4c197369680edbb5e6ce5888b1 100644 (file)
--- 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);