Index: /home/malcolm/svn/svn-trunk/subversion/include/svn_error_codes.h =================================================================== --- /home/malcolm/svn/svn-trunk/subversion/include/svn_error_codes.h (revision 21346) +++ /home/malcolm/svn/svn-trunk/subversion/include/svn_error_codes.h (working copy) @@ -587,6 +587,11 @@ 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: /home/malcolm/svn/svn-trunk/subversion/include/svn_fs.h =================================================================== --- /home/malcolm/svn/svn-trunk/subversion/include/svn_fs.h (revision 21346) +++ /home/malcolm/svn/svn-trunk/subversion/include/svn_fs.h (working copy) @@ -1411,7 +1411,8 @@ * * 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: /home/malcolm/svn/svn-trunk/subversion/libsvn_fs_fs/fs_fs.c =================================================================== --- /home/malcolm/svn/svn-trunk/subversion/libsvn_fs_fs/fs_fs.c (revision 21346) +++ /home/malcolm/svn/svn-trunk/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 */ @@ -201,6 +202,12 @@ } 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); @@ -3001,6 +3012,85 @@ return SVN_NO_ERROR; } +/* Unlock the prototype revision file for transaction TXN_ID in + filesystem FS. Perform temporary allocations in POOL. */ +static svn_error_t * +unlock_proto_rev(svn_fs_t *fs, const char *txn_id, apr_pool_t *pool) +{ + return svn_io_remove_file(path_txn_proto_rev_lock(fs, txn_id, pool), pool); +} + +/* 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, or return SVN_FS_REP_BEING_WRITTEN + if the prototype revision file is already locked. + + IS_COMMIT indicates whether the lock is being requested for a commit + or a representation write, and is used solely to determine the text + of the error message generated when returning SVN_FS_REP_BEING_WRITTEN. + + Perform all allocations in POOL. */ +static svn_error_t * +get_writable_proto_rev(apr_file_t **file, + svn_fs_t *fs, const char *txn_id, + svn_boolean_t is_commit, + apr_pool_t *pool) +{ + svn_error_t *err; + apr_off_t offset; + + /* Create the proto-rev lock file to ensure that no-one else is + writing to the proto-rev file. We could instead choose to use a + more complex combination of fcntl() locking and a mutex, like we + do for the fs-wide write-lock, but we'd need to create a mutex + per transaction. In practice, there's no measurable impact in + creating and deleting this file per representation written, and + one advantage of using a persistent file is that the transaction + becomes permanently uncommittable should the caller abort without + completely writing the representation. */ + err = svn_io_file_open(file, path_txn_proto_rev_lock(fs, txn_id, pool), + APR_WRITE | APR_CREATE | APR_EXCL, APR_OS_DEFAULT, + pool); + if (err) + { + if (! APR_STATUS_IS_EEXIST(err->apr_err)) + return err; + + svn_error_clear(err); + return svn_error_create(SVN_ERR_FS_REP_BEING_WRITTEN, NULL, + is_commit ? + _("Cannot commit a transaction while a representation is being written") : + _("Cannot write a new representation while a previous representation is being written")); + } + SVN_ERR(svn_io_file_close(*file, pool)); + + /* Now open the prototype revision file and sync 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); + if (err) + { + svn_error_clear(unlock_proto_rev(fs, txn_id, pool)); + return err; + } + + /* 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; + err = svn_io_file_seek(*file, APR_END, &offset, 0); + if (err) + { + svn_error_clear(unlock_proto_rev(fs, txn_id, pool)); + return err; + } + + return SVN_NO_ERROR; +} + /* This baton is used by the representation writing streams. It keeps track of the checksum information as well as the total size of the representation so far. */ @@ -3117,10 +3207,9 @@ { 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; @@ -3136,19 +3225,9 @@ 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, + fs, svn_fs_fs__id_txn_id(noderev->id), + FALSE /* is not for a commit */, b->pool)); b->file = file; b->rep_stream = svn_stream_from_aprfile(file, b->pool); @@ -3230,6 +3309,7 @@ b->pool)); SVN_ERR(svn_io_file_close(b->file, b->pool)); + SVN_ERR(unlock_proto_rev(b->fs, rep->txn_id, b->pool)); svn_pool_destroy(b->pool); return SVN_NO_ERROR; @@ -3923,7 +4003,7 @@ 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; + apr_off_t changed_path_offset; char *buf; apr_hash_t *txnprops; svn_string_t date; @@ -3951,15 +4031,8 @@ 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, cb->fs, cb->txn->id, + TRUE /* is for a commit */, pool)); /* Write out all the node-revisions and directory contents. */ root_id = svn_fs_fs__id_txn_create("0", "0", cb->txn->id, pool); @@ -4001,6 +4074,7 @@ /* 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)); Index: /home/malcolm/svn/svn-trunk/subversion/libsvn_fs_fs/structure =================================================================== --- /home/malcolm/svn/svn-trunk/subversion/libsvn_fs_fs/structure (revision 21346) +++ /home/malcolm/svn/svn-trunk/subversion/libsvn_fs_fs/structure (working copy) @@ -217,6 +217,8 @@ rev Prototype rev file with new text reps props Transaction props + rev-lock Temporary file created to indicate that + the prototype rev file is being written to next-ids Next temporary node-ID and copy-ID changes Changed-path information so far node.. New node-rev data for node @@ -225,6 +227,11 @@ The two kinds of props files are both in hash dump format. +The "rev-lock" file exists only while data is being written to the +prototype rev file. The filesystem uses the existence of this file +to ensure that each write operation has completed before the next is +allowed to start. + The "next-ids" file contains a single line " \n" giving the next temporary node-ID and copy-ID assignments (without the leading underscores).