[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Fixing the FSFS corruption bug, again

From: Malcolm Rowe <malcolm-svn-dev_at_farside.org.uk>
Date: 2006-10-02 13:20:03 CEST

Here's an updated patch to (hopefully) fix the FSFS corruption problem,
this time using a method that works on NFS filesystems.

See http://svn.haxx.se/dev/archive-2006-09/0078.shtml and issue #2467
for background.

Comments would be appreciated, as would test results from OS X or Windows.

Regards,
Malcolm

[[[
FSFS: Detect a particular class of FS API violation (encountered while
modifying a transaction) and return an error instead of corrupting the
eventual revision file.

(There is evidence that suggests that mod_dav_svn is currently committing
this API violation; see issue 2467, of which this may be the root cause)

Prevent concurrent writes to the prototype revision file (whether by
writing a representation or committing the transaction) by:

1. Introducing a framework for storing per-filesystem and per-transaction
   shared data (i.e. data that is shared between all threads in a
   process).

2. Using that framework to check whether any threads in the current
   process are writing to the proto-revision file.

3. Creating a 'rev-lock' file in the transaction directory to check
   whether any other processes are writing to the proto-revision file.

* subversion/include/svn_fs.h
  (svn_fs_apply_text): Document the requirement that the caller close
    the returned stream before further operating on the transaction,
    using the same wording as svn_fs_apply_textdelta() already provides.

* subversion/include/svn_error_codes.h
  (SVN_ERR_FS_REP_BEING_WRITTEN): New.

* subversion/libsvn_fs_fs/fs.h
  (fs_fs_shared_txn_data_t): New. Shared (one per process) object to
    hold shared data for an active transaction in a filesystem.
  (fs_fs_shared_data_t): New. Shared (one per process) object to hold
    shared data for a filesystem.
  (fs_fs_data_t): Extend to contain a reference to an explicit
    fs_fs_shared_data_t object rather that just the single write-lock
    mutex.

* subversion/libsvn_fs_fs/fs.c
  (SVN_FSFS_LOCK_USERDATA_PREFIX): Remove, in favour of the more general...
  (SVN_FSFS_SHARED_USERDATA_PREFIX): New.
  (fs_serialized_init): Allocate an explicit 'shared filesystem data'
    object, and store that in the userdata pool rather than just the
    write-lock mutex. Initialise the contents of that object, including
    the mutexes to serialise write access to the filesystem and read/write
    access to the transaction list.

* subversion/libsvn_fs_fs/structure
  (Transaction layout) Document the new 'rev-lock' file and locking
    mechanism.

* subversion/libsvn_fs_fs/fs_fs.c
  (PATH_REV_LOCK, path_txn_proto_rev_lock): New.

  (get_shared_txn, free_shared_txn): New. Add and remove transactions
    from the shared transaction list.
  (with_txnlist_lock): New. Run a function while holding the lock for
    the shared transaction list.

  (struct unlock_proto_rev_baton, unlock_proto_rev_body,
   unlock_proto_rev):
    New. Unlock the prototype revision file.
  (unlock_proto_rev_list_locked): New. Ditto, but for the case where
    the shared transaction list is already locked.
  (struct get_writable_proto_rev_baton, get_writable_proto_rev_body,
   get_writable_proto_rev): New. Lock the prototype revision file and
    return a writable file handle to it.
  (purge_shared_txn_body, purge_shared_txn): New. Remove a transaction
    from the shared transaction list.

  (svn_fs_fs__create_txn): Create an empty 'rev-lock' file when creating
    a transaction.
  (svn_fs_fs__purge_txn): Remove the shared transaction object before
    removing the transaction directory.

  (struct rep_write_baton): Keep track of the returned lock cookie from
    get_writable_proto_rev() so that the proto-rev file can be unlocked.
  (rep_write_get_baton): Call get_writable_proto_rev() to open the
    proto-rev file, stashing the cookie returned into the rep_write_baton).
  (rep_write_contents_close): Unlock the proto-rev file using the cookie.

  (svn_fs_fs__with_write_lock): Account for the movement of the
    filesystem write-lock mutex into the explicit per-filesystem object.

  (commit_body): Call get_writable_proto_rev() to open the proto-rev file,
    and call unlock_proto_rev() to unlock it after we've finalised it.
]]]

Index: subversion/include/svn_fs.h
===================================================================
--- subversion/include/svn_fs.h (revision 21732)
+++ subversion/include/svn_fs.h (working copy)
@@ -1415,7 +1415,8 @@ svn_error_t *svn_fs_apply_textdelta(svn_
  *
  * Set @a *contents_p to a stream ready to receive full textual data.
  * When the caller closes this stream, the data replaces the previous
- * contents of the file.
+ * contents of the file. The caller must write all file data and close
+ * the stream before making further changes to the transaction.
  *
  * If @a path does not exist in @a root, return an error. (You cannot use
  * this routine to create new files; use svn_fs_make_file() to create
Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h (revision 21732)
+++ subversion/include/svn_error_codes.h (working copy)
@@ -587,6 +587,11 @@ SVN_ERROR_START
              SVN_ERR_FS_CATEGORY_START + 43,
              "Unsupported FS format")
 
+ /** @since New in 1.5. */
+ SVN_ERRDEF(SVN_ERR_FS_REP_BEING_WRITTEN,
+ SVN_ERR_FS_CATEGORY_START + 44,
+ "Representation is being written")
+
   /* repos errors */
 
   SVN_ERRDEF(SVN_ERR_REPOS_LOCKED,
Index: subversion/libsvn_fs_fs/fs.h
===================================================================
--- subversion/libsvn_fs_fs/fs.h (revision 21732)
+++ subversion/libsvn_fs_fs/fs.h (working copy)
@@ -45,6 +45,65 @@ extern "C" {
 #define NUM_DIR_CACHE_ENTRIES 128
 #define DIR_CACHE_ENTRIES_MASK(x) ((x) & (NUM_DIR_CACHE_ENTRIES - 1))
 
+
+/* Private FSFS-specific data shared between all svn_txn_t objects that
+ relate to a particular transaction in a filesystem (as identified
+ by transaction id and filesystem UUID). Objects of this type are
+ allocated in their own subpool of the common pool. */
+struct fs_fs_shared_txn_data_t;
+typedef struct fs_fs_shared_txn_data_t
+{
+ /* The next transaction in the list, or NULL if there is no following
+ transaction. */
+ struct fs_fs_shared_txn_data_t *next;
+
+ /* This transaction's ID. This is in the form <rev>-<uniqueifier>,
+ where <uniqueifier> runs from 0-99999 (see create_txn_dir() in
+ fs_fs.c). */
+ char txn_id[18];
+
+ /* Whether the transaction's prototype revision file is locked for
+ writing by any thread in this process (including the current
+ thread; recursive locks are not permitted). This is effectively
+ a non-recursive mutex. */
+ svn_boolean_t being_written;
+
+ /* The pool in which this object has been allocated; a subpool of the
+ common pool. */
+ apr_pool_t *pool;
+} fs_fs_shared_txn_data_t;
+
+
+/* Private FSFS-specific data shared between all svn_fs_t objects that
+ relate to a particular filesystem, as identified by filesystem UUID.
+ Objects of this type are allocated in the common pool. */
+typedef struct
+{
+ /* A list of shared transaction objects for each transaction that is
+ currently active, or NULL if none are. All access to this list,
+ including the contents of the objects stored in it, is synchronised
+ under TXN_LIST_LOCK. */
+ fs_fs_shared_txn_data_t *txns;
+
+ /* A free transaction object, or NULL if there is no free object.
+ Access to this object is synchronised under TXN_LIST_LOCK. */
+ fs_fs_shared_txn_data_t *free_txn;
+
+#if APR_HAS_THREADS
+ /* A lock for intra-process synchronization when accessing the TXNS list. */
+ apr_thread_mutex_t *txn_list_lock;
+
+ /* A lock for intra-process synchronization when grabbing the
+ repository write lock. */
+ apr_thread_mutex_t *fs_write_lock;
+#endif
+
+ /* The common pool, under which this object is allocated, subpools
+ of which are used to allocate the transaction objects. */
+ apr_pool_t *common_pool;
+} fs_fs_shared_data_t;
+
+/* Private (non-shared) FSFS-specific data for each svn_fs_t object. */
 typedef struct
 {
   /* A cache of the last directory opened within the filesystem. */
@@ -58,12 +117,8 @@ typedef struct
   /* The uuid of this FS. */
   const char *uuid;
 
-#if APR_HAS_THREADS
- /* A lock for intra-process synchronization when grabbing the
- repository write lock. Common to all repositories with the same
- uuid; discovered using the serialized_init function. */
- apr_thread_mutex_t *lock;
-#endif
+ /* Data shared between all svn_fs_t objects for a given filesystem. */
+ fs_fs_shared_data_t *shared;
 } fs_fs_data_t;
 
 
Index: subversion/libsvn_fs_fs/fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs.c (revision 21732)
+++ subversion/libsvn_fs_fs/fs.c (working copy)
@@ -1,7 +1,7 @@
 /* fs.c --- creating, opening and closing filesystems
  *
  * ====================================================================
- * Copyright (c) 2000-2004 CollabNet. All rights reserved.
+ * Copyright (c) 2000-2006 CollabNet. All rights reserved.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -39,9 +39,9 @@
 #include "../libsvn_fs/fs-loader.h"
 
 /* A prefix for the pool userdata variables used to hold
- per-repository mutexes. See fs_serialized_init. */
+ per-filesystem shared data. See fs_serialized_init. */
 #if APR_HAS_THREADS
-#define SVN_FSFS_LOCK_USERDATA_PREFIX "svn-fsfs-lock-"
+#define SVN_FSFS_SHARED_USERDATA_PREFIX "svn-fsfs-shared-"
 #endif
 
 
@@ -63,52 +63,65 @@ check_already_open(svn_fs_t *fs)
 static svn_error_t *
 fs_serialized_init(svn_fs_t *fs, apr_pool_t *common_pool, apr_pool_t *pool)
 {
-#if APR_HAS_THREADS
   fs_fs_data_t *ffd = fs->fsap_data;
   const char *key;
   void *val;
- apr_thread_mutex_t *lock;
+ fs_fs_shared_data_t *ffsd;
   apr_status_t status;
 
- /* POSIX fcntl locks are per-process, so we need a mutex for
- intra-process synchronization when grabbing the repository write
- lock. Fetch a repository-specific lock from the pool user data.
-
- Note that we are allocating a small amount of long-lived data for
+ /* Note that we are allocating a small amount of long-lived data for
      each separate repository opened during the lifetime of the
      svn_fs_initialize pool. It's unlikely that anyone will notice
- this modest expenditure; the alternative is to put each lock into
- a reference-counted structure allocated in a subpool, and add a
- serialized deconstructor to the FS vtable. That's more machinery
- than it's worth.
+ the modest expenditure; the alternative is to allocate each structure
+ in a subpool, add a reference-count, and add a serialized deconstructor
+ to the FS vtable. That's more machinery than it's worth.
 
      Using the uuid to obtain the lock creates a corner case if a
      caller uses svn_fs_set_uuid on the repository in a process where
      other threads might be using the same repository through another
      FS object. The only real-world consumer of svn_fs_set_uuid is
      "svnadmin load", so this is a low-priority problem, and we don't
- know of a better way of associating a mutex with the
+ know of a better way of associating such data with the
      repository. */
 
- key = apr_pstrcat(pool, SVN_FSFS_LOCK_USERDATA_PREFIX, ffd->uuid,
+ key = apr_pstrcat(pool, SVN_FSFS_SHARED_USERDATA_PREFIX, ffd->uuid,
                     (char *) NULL);
   status = apr_pool_userdata_get(&val, key, common_pool);
   if (status)
- return svn_error_wrap_apr(status, _("Can't fetch FSFS mutex"));
- lock = val;
- if (!lock)
+ return svn_error_wrap_apr(status, _("Can't fetch FSFS shared data"));
+ ffsd = val;
+
+ if (!ffsd)
     {
- status = apr_thread_mutex_create(&lock, APR_THREAD_MUTEX_DEFAULT,
- common_pool);
+ ffsd = apr_pcalloc(common_pool, sizeof(*ffsd));
+ ffsd->common_pool = common_pool;
+
+#if APR_HAS_THREADS
+ /* POSIX fcntl locks are per-process, so we need a mutex for
+ intra-process synchronization when grabbing the repository write
+ lock. */
+ status = apr_thread_mutex_create(&ffsd->fs_write_lock,
+ APR_THREAD_MUTEX_DEFAULT, common_pool);
       if (status)
- return svn_error_wrap_apr(status, _("Can't create FSFS mutex"));
+ return svn_error_wrap_apr(status,
+ _("Can't create FSFS write-lock mutex"));
+
+ /* We also need a mutex for synchronising access to the active
+ transaction list and free transaction pointer. */
+ status = apr_thread_mutex_create(&ffsd->txn_list_lock,
+ APR_THREAD_MUTEX_DEFAULT, common_pool);
+ if (status)
+ return svn_error_wrap_apr(status,
+ _("Can't create FSFS txn list mutex"));
+#endif
+
       key = apr_pstrdup(common_pool, key);
- status = apr_pool_userdata_set(lock, key, NULL, common_pool);
+ status = apr_pool_userdata_set(ffsd, key, NULL, common_pool);
       if (status)
- return svn_error_wrap_apr(status, _("Can't store FSFS mutex"));
+ return svn_error_wrap_apr(status, _("Can't store FSFS shared data"));
     }
- ffd->lock = lock;
-#endif
+
+ ffd->shared = ffsd;
 
   return SVN_NO_ERROR;
 }
Index: subversion/libsvn_fs_fs/structure
===================================================================
--- subversion/libsvn_fs_fs/structure (revision 21732)
+++ subversion/libsvn_fs_fs/structure (working copy)
@@ -216,6 +216,7 @@ Transaction layout
 A transaction directory has the following layout:
 
   rev Prototype rev file with new text reps
+ rev-lock Lockfile for writing to the above
   props Transaction props
   next-ids Next temporary node-ID and copy-ID
   changes Changed-path information so far
@@ -223,6 +224,11 @@ A transaction directory has the followin
   node.<nid>.<cid>.props Props for new node-rev, if changed
   node.<nid>.<cid>.children Directory contents for node-rev
 
+The prototype rev file is used to store the text representations as
+they are received from the client. To ensure that only one client is
+writing to the file at a given time, the "rev-lock" file is locked for
+the duration of each write.
+
 The two kinds of props files are both in hash dump format.
 
 The "next-ids" file contains a single line "<next-temp-node-id>
Index: subversion/libsvn_fs_fs/fs_fs.c
===================================================================
--- subversion/libsvn_fs_fs/fs_fs.c (revision 21732)
+++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
@@ -71,6 +71,7 @@
 #define PATH_TXN_PROPS "props" /* Transaction properties */
 #define PATH_NEXT_IDS "next-ids" /* Next temporary ID assignments */
 #define PATH_REV "rev" /* Proto rev file */
+#define PATH_REV_LOCK "rev-lock" /* Proto rev (write) lock file */
 #define PATH_PREFIX_NODE "node." /* Prefix for node filename */
 #define PATH_EXT_TXN ".txn" /* Extension of txn dir */
 #define PATH_EXT_CHILDREN ".children" /* Extension for dir contents */
@@ -202,6 +203,12 @@ path_txn_proto_rev(svn_fs_t *fs, const c
 }
 
 static const char *
+path_txn_proto_rev_lock(svn_fs_t *fs, const char *txn_id, apr_pool_t *pool)
+{
+ return svn_path_join(path_txn_dir(fs, txn_id, pool), PATH_REV_LOCK, pool);
+}
+
+static const char *
 path_txn_node_rev(svn_fs_t *fs, const svn_fs_id_t *id, apr_pool_t *pool)
 {
   const char *txn_id = svn_fs_fs__id_txn_id(id);
@@ -227,6 +234,342 @@ path_txn_node_children(svn_fs_t *fs, con
                      PATH_EXT_CHILDREN, NULL);
 }
 
+
+
+/* Functions for working with shared transaction data. */
+
+/* Return the transaction object for transaction TXN_ID from the
+ transaction list of filesystem FS (which must already be locked via the
+ txn_list_lock mutex). If the transaction does not exist in the list,
+ then create a new transaction object and return it (if CREATE_NEW is
+ true) or return NULL (otherwise). */
+static fs_fs_shared_txn_data_t *
+get_shared_txn(svn_fs_t *fs, const char *txn_id, svn_boolean_t create_new)
+{
+ fs_fs_data_t *ffd = fs->fsap_data;
+ fs_fs_shared_data_t *ffsd = ffd->shared;
+ fs_fs_shared_txn_data_t *txn;
+
+ for (txn = ffsd->txns; txn; txn = txn->next)
+ if (strcmp(txn->txn_id, txn_id) == 0)
+ break;
+
+ if (txn || !create_new)
+ return txn;
+
+ /* Use the transaction object from the (single-object) freelist,
+ if one is available, or otherwise create a new object. */
+ if (ffsd->free_txn)
+ {
+ txn = ffsd->free_txn;
+ ffsd->free_txn = NULL;
+ }
+ else
+ {
+ apr_pool_t *subpool = svn_pool_create(ffsd->common_pool);
+ txn = apr_palloc(subpool, sizeof(*txn));
+ txn->pool = subpool;
+ }
+
+ assert(strlen(txn_id) < sizeof(txn->txn_id));
+ strcpy(txn->txn_id, txn_id);
+ txn->being_written = FALSE;
+
+ /* Link this transaction into the head of the list. We will typically
+ be dealing with only one active transaction at a time, so it makes
+ sense for searches through the transaction list to look at the
+ newest transactions first. */
+ txn->next = ffsd->txns;
+ ffsd->txns = txn;
+
+ return txn;
+}
+
+/* Free the transaction object for transaction TXN_ID, and remove it
+ from the transaction list of filesystem FS (which must already be
+ locked via the txn_list_lock mutex). Do nothing if the transaction
+ does not exist. */
+static void
+free_shared_txn(svn_fs_t *fs, const char *txn_id)
+{
+ fs_fs_data_t *ffd = fs->fsap_data;
+ fs_fs_shared_data_t *ffsd = ffd->shared;
+ fs_fs_shared_txn_data_t *txn, *prev = NULL;
+
+ for (txn = ffsd->txns; txn; prev = txn, txn = txn->next)
+ if (strcmp(txn->txn_id, txn_id) == 0)
+ break;
+
+ if (!txn)
+ return;
+
+ if (prev)
+ prev->next = txn->next;
+ else
+ ffsd->txns = txn->next;
+
+ /* As we typically will be dealing with one transaction after another,
+ we will maintain a single-object free list so that we can hopefully
+ keep reusing the same transaction object. */
+ if (!ffsd->free_txn)
+ ffsd->free_txn = txn;
+ else
+ svn_pool_destroy(txn->pool);
+}
+
+
+/* Obtain a lock on the transaction list of filesystem FS, call BODY
+ with FS, BATON, and POOL, and then unlock the transaction list.
+ Return what BODY returned. */
+static svn_error_t *
+with_txnlist_lock(svn_fs_t *fs,
+ svn_error_t *(*body)(svn_fs_t *fs,
+ void *baton,
+ apr_pool_t *pool),
+ void *baton,
+ apr_pool_t *pool)
+{
+ svn_error_t *err;
+#if APR_HAS_THREADS
+ fs_fs_data_t *ffd = fs->fsap_data;
+ fs_fs_shared_data_t *ffsd = ffd->shared;
+ apr_status_t apr_err;
+
+ apr_err = apr_thread_mutex_lock(ffsd->txn_list_lock);
+ if (apr_err)
+ return svn_error_wrap_apr(apr_err, _("Can't grab FSFS txn list mutex"));
+#endif
+
+ err = body(fs, baton, pool);
+
+#if APR_HAS_THREADS
+ apr_err = apr_thread_mutex_unlock(ffsd->txn_list_lock);
+ if (apr_err && !err)
+ return svn_error_wrap_apr(apr_err, _("Can't ungrab FSFS txn list mutex"));
+#endif
+
+ return err;
+}
+
+/* A structure used by unlock_proto_rev() and unlock_proto_rev_body(),
+ which see. */
+struct unlock_proto_rev_baton
+{
+ const char *txn_id;
+ void *lockcookie;
+};
+
+/* Callback used in the implementation of unlock_proto_rev(). */
+static svn_error_t *
+unlock_proto_rev_body(svn_fs_t *fs, void *baton, apr_pool_t *pool)
+{
+ struct unlock_proto_rev_baton *b = baton;
+ const char *txn_id = b->txn_id;
+ apr_file_t *lockfile = b->lockcookie;
+ fs_fs_shared_txn_data_t *txn = get_shared_txn(fs, txn_id, FALSE);
+ apr_status_t apr_err;
+
+ if (!txn)
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+ _("Can't unlock unknown transaction '%s'"),
+ txn_id);
+ if (!txn->being_written)
+ return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+ _("Can't unlock nonlocked transaction '%s'"),
+ txn_id);
+
+ apr_err = apr_file_unlock(lockfile);
+ if (apr_err)
+ return svn_error_wrap_apr
+ (apr_err,
+ _("Can't unlock prototype revision lockfile for transaction '%s'"),
+ txn_id);
+ apr_err = apr_file_close(lockfile);
+ if (apr_err)
+ return svn_error_wrap_apr
+ (apr_err,
+ _("Can't close prototype revision lockfile for transaction '%s'"),
+ txn_id);
+
+ txn->being_written = FALSE;
+
+ return SVN_NO_ERROR;
+}
+
+/* Unlock the prototype revision file for transaction TXN_ID in filesystem
+ FS using cookie LOCKCOOKIE. The original prototype revision file must
+ have been closed _before_ calling this function.
+
+ Perform temporary allocations in POOL. */
+static svn_error_t *
+unlock_proto_rev(svn_fs_t *fs, const char *txn_id, void *lockcookie,
+ apr_pool_t *pool)
+{
+ struct unlock_proto_rev_baton b;
+
+ b.txn_id = txn_id;
+ b.lockcookie = lockcookie;
+ return with_txnlist_lock(fs, unlock_proto_rev_body, &b, pool);
+}
+
+/* Same as unlock_proto_rev(), but requires that the transaction list
+ lock is already held. */
+static svn_error_t *
+unlock_proto_rev_list_locked(svn_fs_t *fs, const char *txn_id,
+ void *lockcookie,
+ apr_pool_t *pool)
+{
+ struct unlock_proto_rev_baton b;
+
+ b.txn_id = txn_id;
+ b.lockcookie = lockcookie;
+ return unlock_proto_rev_body(fs, &b, pool);
+}
+
+/* A structure used by get_writable_proto_rev() and
+ get_writable_proto_rev_body(), which see. */
+struct get_writable_proto_rev_baton
+{
+ apr_file_t **file;
+ void **lockcookie;
+ const char *txn_id;
+};
+
+/* Callback used in the implementation of get_writable_proto_rev(). */
+static svn_error_t *
+get_writable_proto_rev_body(svn_fs_t *fs, void *baton, apr_pool_t *pool)
+{
+ struct get_writable_proto_rev_baton *b = baton;
+ apr_file_t **file = b->file;
+ void **lockcookie = b->lockcookie;
+ const char *txn_id = b->txn_id;
+ svn_error_t *err;
+ fs_fs_shared_txn_data_t *txn = get_shared_txn(fs, txn_id, TRUE);
+
+ /* First, ensure that no thread in this process (including this one)
+ is currently writing to this transaction's proto-rev file. */
+ if (txn->being_written)
+ return svn_error_createf(SVN_ERR_FS_REP_BEING_WRITTEN, NULL,
+ _("Cannot write to the prototype revision file of transaction '%s' because a previous representation is currently being written by this process"),
+ txn_id);
+
+
+ /* We know that no thread in this process is writing to the proto-rev
+ file, and by extension, that no thread in this process is holding a
+ lock on the prototype revision lock file. It is therefore safe
+ for us to attempt to lock this file, to see if any other process
+ is holding a lock. */
+
+ {
+ apr_file_t *lockfile;
+ apr_status_t apr_err;
+ const char *lockfile_path = path_txn_proto_rev_lock(fs, txn_id, pool);
+
+ /* Open the proto-rev lockfile, creating it if necessary, as it may
+ not exist if the transaction dates from before the lockfiles were
+ introduced.
+
+ ### We'd also like to use something like svn_io_file_lock2(), but
+ that forces us to create a subpool just to be able to unlock
+ the file, which seems a waste. */
+ SVN_ERR(svn_io_file_open(&lockfile, lockfile_path,
+ APR_WRITE | APR_CREATE, APR_OS_DEFAULT, pool));
+
+ apr_err = apr_file_lock(lockfile,
+ APR_FLOCK_EXCLUSIVE | APR_FLOCK_NONBLOCK);
+ if (apr_err)
+ {
+ svn_error_clear(svn_io_file_close(lockfile, pool));
+
+ if (APR_STATUS_IS_EAGAIN(apr_err))
+ return svn_error_createf(SVN_ERR_FS_REP_BEING_WRITTEN, NULL,
+ _("Cannot write to the prototype revision file of transaction '%s' because a previous representation is currently being written by another process"),
+ txn_id);
+
+ return svn_error_wrap_apr(apr_err,
+ _("Can't get exclusive lock on file '%s'"),
+ svn_path_local_style(lockfile_path, pool));
+ }
+
+ *lockcookie = lockfile;
+ }
+
+ /* We've successfully locked the transaction; mark it as such. */
+ txn->being_written = TRUE;
+
+
+ /* Now open the prototype revision file and seek to the end. */
+ err = svn_io_file_open(file, path_txn_proto_rev(fs, txn_id, pool),
+ APR_WRITE | APR_BUFFERED, APR_OS_DEFAULT, pool);
+
+ /* You might expect that we could dispense with the following seek
+ and achieve the same thing by opening the file using APR_APPEND.
+ Unfortunately, APR's buffered file implementation unconditionally
+ places its initial file pointer at the start of the file (even for
+ files opened with APR_APPEND), so we need this seek to reconcile
+ the APR file pointer to the OS file pointer (since we need to be
+ able to read the current file position later). */
+ if (!err)
+ {
+ apr_off_t offset = 0;
+ err = svn_io_file_seek(*file, APR_END, &offset, 0);
+ }
+
+ if (err)
+ {
+ svn_error_clear(unlock_proto_rev_list_locked(fs, txn_id, *lockcookie,
+ pool));
+ *lockcookie = NULL;
+ }
+
+ return err;
+}
+
+/* Get a handle to the prototype revision file for transaction TXN_ID in
+ filesystem FS, and lock it for writing. Return FILE, a file handle
+ positioned at the end of the file, and LOCKCOOKIE, a cookie that
+ should be passed to unlock_proto_rev() to unlock the file once FILE
+ has been closed.
+
+ If the prototype revision file is already locked, return error
+ SVN_FS_REP_BEING_WRITTEN.
+
+ Perform all allocations in POOL. */
+static svn_error_t *
+get_writable_proto_rev(apr_file_t **file,
+ void **lockcookie,
+ svn_fs_t *fs, const char *txn_id,
+ apr_pool_t *pool)
+{
+ struct get_writable_proto_rev_baton b;
+
+ b.file = file;
+ b.lockcookie = lockcookie;
+ b.txn_id = txn_id;
+
+ return with_txnlist_lock(fs, get_writable_proto_rev_body, &b, pool);
+}
+
+/* Callback used in the implementation of purge_shared_txn(). */
+static svn_error_t *
+purge_shared_txn_body(svn_fs_t *fs, void *baton, apr_pool_t *pool)
+{
+ const char *txn_id = baton;
+
+ free_shared_txn(fs, txn_id);
+ return SVN_NO_ERROR;
+}
+
+/* Purge the shared data for transaction TXN_ID in filesystem FS.
+ Perform all allocations in POOL. */
+static svn_error_t *
+purge_shared_txn(svn_fs_t *fs, const char *txn_id, apr_pool_t *pool)
+{
+ return with_txnlist_lock(fs, purge_shared_txn_body, &txn_id, pool);
+}
+
+
+
 /* Fetch the current offset of FILE into *OFFSET_P. */
 static svn_error_t *
 get_file_offset(apr_off_t *offset_p, apr_file_t *file, apr_pool_t *pool)
@@ -2546,6 +2889,10 @@ svn_fs_fs__create_txn(svn_fs_txn_t **txn
   SVN_ERR(svn_io_file_create(path_txn_proto_rev(fs, txn->id, pool), "",
                              pool));
 
+ /* Create an empty rev-lock file. */
+ SVN_ERR(svn_io_file_create(path_txn_proto_rev_lock(fs, txn->id, pool), "",
+ pool));
+
   /* Create an empty changes file. */
   SVN_ERR(svn_io_file_create(path_txn_changes(fs, txn->id, pool), "",
                              pool));
@@ -2761,6 +3108,8 @@ svn_fs_fs__purge_txn(svn_fs_t *fs,
                      const char *txn_id,
                      apr_pool_t *pool)
 {
+ /* Remove the shared transaction object associated with this transaction. */
+ SVN_ERR(purge_shared_txn(fs, txn_id, pool));
   /* Remove the directory associated with this transaction. */
   return svn_io_remove_dir(path_txn_dir(fs, txn_id, pool), pool);
 }
@@ -3042,6 +3391,9 @@ struct rep_write_baton
 
   /* Actual output file. */
   apr_file_t *file;
+ /* Lock 'cookie' used to unlock the output file once we've finished
+ writing to it. */
+ void *lockcookie;
 
   struct apr_md5_ctx_t md5_context;
 
@@ -3129,10 +3481,9 @@ rep_write_get_baton(struct rep_write_bat
 {
   struct rep_write_baton *b;
   apr_file_t *file;
- apr_off_t offset;
   representation_t *base_rep;
   svn_stream_t *source;
- const char *txn_id, *header;
+ const char *header;
   svn_txdelta_window_handler_t wh;
   void *whb;
   fs_fs_data_t *ffd = fs->fsap_data;
@@ -3148,19 +3499,9 @@ rep_write_get_baton(struct rep_write_bat
   b->noderev = noderev;
 
   /* Open the prototype rev file and seek to its end. */
- txn_id = svn_fs_fs__id_txn_id(noderev->id);
- SVN_ERR(svn_io_file_open(&file, path_txn_proto_rev(fs, txn_id, b->pool),
- APR_WRITE | APR_BUFFERED,
- APR_OS_DEFAULT, b->pool));
- /* You might expect that we could dispense with this seek and achieve
- the same thing by opening the file using APR_APPEND. Unfortunately,
- APR's buffered file implementation unconditionally places its initial
- file pointer at the start of the file (even for files opened with
- APR_APPEND), so we'd always need this seek to reconcile the APR
- file pointer to the OS file pointer (since we need to be able to
- read the current file position later). */
- offset = 0;
- SVN_ERR(svn_io_file_seek(file, APR_END, &offset, 0));
+ SVN_ERR(get_writable_proto_rev(&file, &b->lockcookie,
+ fs, svn_fs_fs__id_txn_id(noderev->id),
+ b->pool));
 
   b->file = file;
   b->rep_stream = svn_stream_from_aprfile(file, b->pool);
@@ -3242,6 +3583,7 @@ rep_write_contents_close(void *baton)
                                        b->pool));
 
   SVN_ERR(svn_io_file_close(b->file, b->pool));
+ SVN_ERR(unlock_proto_rev(b->fs, rep->txn_id, b->lockcookie, b->pool));
   svn_pool_destroy(b->pool);
 
   return SVN_NO_ERROR;
@@ -3810,11 +4152,12 @@ svn_fs_fs__with_write_lock(svn_fs_t *fs,
 
 #if APR_HAS_THREADS
   fs_fs_data_t *ffd = fs->fsap_data;
+ fs_fs_shared_data_t *ffsd = ffd->shared;
   apr_status_t status;
 
   /* POSIX fcntl locks are per-process, so we need to serialize locks
      within the process. */
- status = apr_thread_mutex_lock(ffd->lock);
+ status = apr_thread_mutex_lock(ffsd->fs_write_lock);
   if (status)
     return svn_error_wrap_apr(status, _("Can't grab FSFS repository mutex"));
 #endif
@@ -3827,7 +4170,7 @@ svn_fs_fs__with_write_lock(svn_fs_t *fs,
   svn_pool_destroy(subpool);
 
 #if APR_HAS_THREADS
- status = apr_thread_mutex_unlock(ffd->lock);
+ status = apr_thread_mutex_unlock(ffsd->fs_write_lock);
   if (status && !err)
     return svn_error_wrap_apr(status,
                               _("Can't ungrab FSFS repository mutex"));
@@ -3935,7 +4278,8 @@ commit_body(void *baton, apr_pool_t *poo
   const char *start_node_id, *start_copy_id;
   svn_revnum_t old_rev, new_rev;
   apr_file_t *proto_file;
- apr_off_t changed_path_offset, offset;
+ void *proto_file_lockcookie;
+ apr_off_t changed_path_offset;
   char *buf;
   apr_hash_t *txnprops;
   svn_string_t date;
@@ -3963,15 +4307,8 @@ commit_body(void *baton, apr_pool_t *poo
   new_rev = old_rev + 1;
 
   /* Get a write handle on the proto revision file. */
- proto_filename = path_txn_proto_rev(cb->fs, cb->txn->id, pool);
- SVN_ERR(svn_io_file_open(&proto_file, proto_filename,
- APR_WRITE | APR_BUFFERED,
- APR_OS_DEFAULT, pool));
- /* Seek to the end of the proto revision file (we can't use
- APR_APPEND to achieve the same thing; see the detailed comment
- in rep_write_get_baton() above). */
- offset = 0;
- SVN_ERR(svn_io_file_seek(proto_file, APR_END, &offset, pool));
+ SVN_ERR(get_writable_proto_rev(&proto_file, &proto_file_lockcookie,
+ cb->fs, cb->txn->id, pool));
 
   /* Write out all the node-revisions and directory contents. */
   root_id = svn_fs_fs__id_txn_create("0", "0", cb->txn->id, pool);
@@ -3992,6 +4329,7 @@ commit_body(void *baton, apr_pool_t *poo
   SVN_ERR(svn_io_file_flush_to_disk(proto_file, pool));
   
   SVN_ERR(svn_io_file_close(proto_file, pool));
+ SVN_ERR(unlock_proto_rev(cb->fs, cb->txn->id, proto_file_lockcookie, pool));
 
   /* Remove any temporary txn props representing 'flags'. */
   SVN_ERR(svn_fs_fs__txn_proplist(&txnprops, cb->txn, pool));
@@ -4013,6 +4351,7 @@ commit_body(void *baton, apr_pool_t *poo
   /* Move the finished rev file into place. */
   old_rev_filename = svn_fs_fs__path_rev(cb->fs, old_rev, pool);
   rev_filename = svn_fs_fs__path_rev(cb->fs, new_rev, pool);
+ proto_filename = path_txn_proto_rev(cb->fs, cb->txn->id, pool);
   SVN_ERR(svn_fs_fs__move_into_place(proto_filename, rev_filename,
                                      old_rev_filename, pool));
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Oct 2 13:20:29 2006

This is an archived mail posted to the Subversion Dev mailing list.