(I wasn't going to comment on this since I don't know this part of the
code too well, but given the crickets…)
So, the undesired behaviour is this:
> This commit failed unexpectedly:
> Traceback (most recent call last):
> File "/home/julianfoad/tmp/svn/wd-nv-7983/test-release-proto-rev-lock-7/test-commits.py", line 59, in <module>
> fs.commit_txn(txn2)
> File "/home/julianfoad/build/subversion-c/subversion/bindings/swig/python/libsvn/fs.py", line 324, in svn_fs_commit_txn
> return _fs.svn_fs_commit_txn(*args)
> svn.core.SubversionException:
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs/fs-loader.c:885
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs_fs/tree.c:2358
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs_fs/transaction.c:4010
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs_fs/fs_fs.c:368
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs_fs/fs_fs.c:240
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs_fs/fs_fs.c:228
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs_fs/transaction.c:3813
> at /home/julianfoad/src/subversion-c/subversion/libsvn_fs_fs/transaction.c:475
> 2 - Can't open file '/home/julianfoad/tmp/svn/wd-nv-7983/repo/db/txn-protorevs/1-a.rev': No such file or directory
> at /home/julianfoad/src/subversion-c/subversion/libsvn_subr/io.c:3931
> + echo 'FAILED: sub-test 9'
> FAILED: sub-test 9
First of all, you said the error message read "already locked". The
error above is ENOENT. Can you clarify this?
The change proposed is this:
Julian Foad wrote on Wed, 10 Jun 2020 17:10 +0100:
> @@ -3881,30 +3898,32 @@ commit_body(void *baton, apr_pool_t *poo
> /* Move the finished rev file into place.
>
> ### This "breaks" the transaction by removing the protorev file
> ### but the revision is not yet complete. If this commit does
> ### not complete for any reason the transaction will be lost. */
> old_rev_filename = svn_fs_fs__path_rev_absolute(cb->fs, old_rev, pool);
> rev_filename = svn_fs_fs__path_rev(cb->fs, new_rev, pool);
> proto_filename = svn_fs_fs__path_txn_proto_rev(cb->fs, txn_id, pool);
> - SVN_ERR(svn_fs_fs__move_into_place(proto_filename, rev_filename,
> + ERR_PROTO_REV_LOCKED(svn_fs_fs__move_into_place(proto_filename, rev_filename,
> old_rev_filename, ffd->flush_to_disk,
> pool));
> + TEST_COMMIT_FAIL(9);
>
> /* Now that we've moved the prototype revision file out of the way,
> we can unlock it (since further attempts to write to the file
> will fail as it no longer exists). We must do this so that we can
> remove the transaction directory later. */
> SVN_ERR(unlock_proto_rev(cb->fs, txn_id, proto_file_lockcookie, pool));
> @@ -3939,12 +3958,20 @@ commit_body(void *baton, apr_pool_t *poo
> SVN_ERR(promote_cached_directories(cb->fs, directory_ids, pool));
>
> /* Remove this transaction directory. */
> SVN_ERR(svn_fs_fs__purge_txn(cb->fs, cb->txn->id, pool));
>
> return SVN_NO_ERROR;
> +
> +
> +on_err_proto_rev_locked:
> + /* If any error occurred while the proto-rev file was writable (locked),
> + * unlock it before returning to clean up as best we can. */
> + err1 = svn_error_compose_create(err1,
> + unlock_proto_rev(cb->fs, txn_id, proto_file_lockcookie, pool));
> + return err1;
> }
>
Why is it correct to call unlock_proto_rev() without checking the error
code svn_fs_fs__move_into_place() returned?
HTH,
Daniel
P.S. Suggest to pass the name of the local variable ERR1 as
a parameter to the macro to avoid action at a distance.
Received on 2020-07-30 01:00:32 CEST