Branko Čibej <br...@wandisco.com> writes: >> Author: kotkov >> Date: Tue Jun 23 14:34:05 2015 >> New Revision: 1687079 >> >> URL: http://svn.apache.org/r1687079 >> Log: >> Only lock the hotcopy destination (by taking db/write-lock, db/pack-lock >> and db/txn-current-lock) when we are running in the incremental mode. We >> don't have to do this in a non-incremental mode, because the destination >> is not openable until we are done with the hotcopy. If it cannot be opened, >> concurrency is not an issue.
[...] > What happens now if you run a non-incremental an an incremental hotcopy > almost in parallel? For the same destination, right? If a non-incremental hotcopy strikes in first, the incremental hotcopy is going to see that there is no db/format file in the destination, and will error out. If these operations happen in reverse order, first incremental hotcopy degrades to non-incremental, and the second, which is also non-incremental, is going to to error out before making it into hotcopy_body(). > Even if that's not an issue, which problem does this change solve? Less locking equals less potential problems. In a non-incremental case, the destination cannot be opened with the FS API until the very last step of the hotcopy, when we atomically put the format file into the filesystem. There is no reason to protect against concurrent changes for something that you cannot open. Here is a theoretically possible exploit for the code before the change: 1) You need to have a Unix process doing parallel non-incremental hotcopies of the same source to two different destinations with svn_fs_hotcopy3() or, perhaps, svn_repos_hotcopy3() calls. 2) The hotcopy source should be an FSFS6 (or less) filesystem without an instance ID. 3) The second svn_fs_hotcopy3() call *is going to block* until the first one is finished. Hotcopy source and destination have same UUIDs and their shared locks, such as fs_fs_shared_data_t.fs_write_lock, clash. If you replace the second hotcopy with another operation — say, with something like svn_repos_fs_commit_txn() or any other call that requires a lock, they are also going to block each other. Regards, Evgeny Kotkov
