Re: Is mod_dav_svn safe for use in a threaded MPM?

Here's an untested port of my patch from 1.4.3 to trunk. I'm
testing this today, and can commit later today or tomorrow if no
one objects.

Index: activity.c
===================================================================
--- activity.c (revision 24424)
+++ activity.c (working copy)
@@ -21,6 +21,9 @@
 #include <httpd.h>
 #include <mod_dav.h>
 
+#include <apr_file_info.h>
+#include <apr_file_io.h>
+
 #include "svn_path.h"
 #include "svn_fs.h"
 #include "svn_repos.h"
@@ -31,44 +34,49 @@
 #define ACTIVITY_DB "dav/activities"
 
 
-const char *
-dav_svn__get_txn(const dav_svn_repos *repos, const char *activity_id)
+static const char *
+_read_txn(const dav_svn_repos *repos, const char *pathname)
 {
- apr_dbm_t *dbm;
   apr_status_t status;
- const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
- const char *txn_name = NULL;
+ apr_finfo_t finfo;
+ char *txn_name = NULL;
+ apr_file_t *activity_file;
+ apr_size_t bytes_read;
 
- pathname = svn_path_join(repos->fs_path, ACTIVITY_DB, repos->pool);
- status = apr_dbm_open(&dbm, pathname, APR_DBM_READONLY,
- APR_OS_DEFAULT, repos->pool);
- if (status != APR_SUCCESS)
+ /* ### let's just assume that any error means the
+ ### activity/transaction doesn't exist */
+# define _CHECK_STATUS if (status != APR_SUCCESS) return NULL
+
+ status = apr_stat(&finfo, pathname, APR_FINFO_SIZE, repos->pool);
+ _CHECK_STATUS;
+
+ status = apr_file_open(&activity_file, pathname, APR_READ | APR_BUFFERED,
+ APR_OS_DEFAULT, repos->pool);
+ _CHECK_STATUS;
+
+ txn_name = apr_palloc(repos->pool, finfo.size + 1); /* null-term'd */
+ status = apr_file_read_full(activity_file, txn_name, finfo.size, &bytes_read);
+ _CHECK_STATUS;
+ if (bytes_read != finfo.size)
     {
- /* ### let's just assume that any error means the DB doesn't exist,
- ### therefore, the activity/transaction doesn't exist */
       return NULL;
     }
+ status = apr_file_close(activity_file);
+ _CHECK_STATUS;
+# undef _CHECK_STATUS
 
- key.dptr = (char *)activity_id;
- key.dsize = strlen(activity_id) + 1; /* null-term'd */
- if (apr_dbm_exists(dbm, key))
- {
- status = apr_dbm_fetch(dbm, key, &value);
- if (status != APR_SUCCESS)
- {
- /* ### again: assume failure means it doesn't exist */
- apr_dbm_close(dbm);
- return NULL;
- }
- txn_name = apr_pstrdup(repos->pool, value.dptr); /* null-term'd */
- apr_dbm_freedatum(dbm, value);
- }
+ txn_name[finfo.size] = '\0';
+ return txn_name;
+}
 
- apr_dbm_close(dbm);
 
- return txn_name;
+const char *
+dav_svn__get_txn(const dav_svn_repos *repos, const char *activity_id)
+{
+ const char *pathname;
+ pathname = svn_path_join_many(repos->pool, repos->fs_path, ACTIVITY_DB,
+ activity_id, NULL);
+ return _read_txn(repos, pathname);
 }
 
 
@@ -76,11 +84,8 @@
 dav_svn__delete_activity(const dav_svn_repos *repos, const char *activity_id)
 {
   dav_error *err = NULL;
- apr_dbm_t *dbm;
   apr_status_t status;
   const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
   svn_fs_txn_t *txn;
   const char *txn_name;
   svn_error_t *serr;
@@ -89,23 +94,11 @@
      404. If the transaction is not present or is immutable, return a
      204. For all other failures, return a 500. */
 
- /* Open the activities database. */
- pathname = svn_path_join(repos->fs_path, ACTIVITY_DB, repos->pool);
- status = apr_dbm_open(&dbm, pathname, APR_DBM_READWRITE,
- APR_OS_DEFAULT, repos->pool);
- if (status != APR_SUCCESS)
- return dav_new_error(repos->pool, HTTP_NOT_FOUND, 0,
- "could not open activities database.");
-
- /* Get the activity from the activity database. */
- key.dptr = (char *)activity_id;
- key.dsize = strlen(activity_id) + 1; /* null-term'd */
- status = apr_dbm_fetch(dbm, key, &value);
- if (status == APR_SUCCESS)
- txn_name = value.dptr;
- else
+ pathname = svn_path_join_many(repos->pool, repos->fs_path, ACTIVITY_DB,
+ activity_id, NULL);
+ txn_name = _read_txn(repos, pathname);
+ if (txn_name == NULL)
     {
- apr_dbm_close(dbm);
       return dav_new_error(repos->pool, HTTP_NOT_FOUND, 0,
                            "could not find activity.");
     }
@@ -131,7 +124,7 @@
               err = dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                          "could not open transaction.",
                                          repos->pool);
- goto cleanup;
+ return err;
             }
         }
       else
@@ -142,24 +135,19 @@
               err = dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
                                          "could not abort transaction.",
                                          repos->pool);
- goto cleanup;
+ return err;
             }
         }
     }
-
+
   /* Finally, we remove the activity from the activities database. */
- status = apr_dbm_delete(dbm, key);
+ status = apr_file_remove(pathname, repos->pool);
   if (status)
     {
       err = dav_new_error(repos->pool, HTTP_INTERNAL_SERVER_ERROR, 0,
                           "unable to remove activity.");
- goto cleanup;
     }
 
- cleanup:
- apr_dbm_freedatum(dbm, value);
- apr_dbm_close(dbm);
-
   return err;
 }
 
@@ -169,37 +157,53 @@
                         const char *activity_id,
                         const char *txn_name)
 {
- apr_dbm_t *dbm;
+ const char *dirname, *pathname;
+ svn_error_t *err;
   apr_status_t status;
- const char *pathname;
- apr_datum_t key;
- apr_datum_t value;
+ apr_file_t *activity_file;
+ apr_size_t nbytes, bytes_written;
 
- pathname = svn_path_join(repos->fs_path, ACTIVITY_DB, repos->pool);
- status = apr_dbm_open(&dbm, pathname, APR_DBM_RWCREATE,
- APR_OS_DEFAULT, repos->pool);
+ /* Create activities directory if it does not yet exist. */
+ dirname = svn_path_join(repos->fs_path, ACTIVITY_DB, repos->pool);
+ err = svn_io_dir_make(dirname, APR_OS_DEFAULT, repos->pool);
+ if (err != NULL && !APR_STATUS_IS_EEXIST(err->apr_err))
+ return dav_svn__convert_err(err, HTTP_INTERNAL_SERVER_ERROR,
+ "could not open files.",
+ repos->pool);
+
+ pathname = svn_path_join_many(repos->pool, repos->fs_path, ACTIVITY_DB,
+ activity_id, NULL);
+ status = apr_file_open(&activity_file, pathname, APR_WRITE | APR_CREATE,
+ APR_OS_DEFAULT, repos->pool);
   if (status != APR_SUCCESS)
     {
       svn_error_t *serr = svn_error_wrap_apr(status, "Can't open activity db");
 
       return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "could not open dbm files.",
+ "could not open files.",
                                   repos->pool);
     }
 
- key.dptr = (char *)activity_id;
- key.dsize = strlen(activity_id) + 1; /* null-term'd */
- value.dptr = (char *)txn_name;
- value.dsize = strlen(txn_name) + 1; /* null-term'd */
- status = apr_dbm_store(dbm, key, value);
- apr_dbm_close(dbm);
+ nbytes = strlen(txn_name);
+ status = apr_file_write_full(activity_file, txn_name, nbytes,
+ &bytes_written);
+ if (status != APR_SUCCESS || bytes_written != nbytes)
+ {
+ svn_error_t *serr =
+ svn_error_wrap_apr(status, "Can't write to activity db");
+ return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
+ "could not write files.",
+ repos->pool);
+ }
+
+ status = apr_file_close(activity_file);
   if (status != APR_SUCCESS)
     {
       svn_error_t *serr =
         svn_error_wrap_apr(status, "Can't close activity db");
 
       return dav_svn__convert_err(serr, HTTP_INTERNAL_SERVER_ERROR,
- "could not close dbm files.",
+ "could not close files.",
                                   repos->pool);
     }
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Apr 5 20:53:39 2007