On 26.02.2013 02:50, Philip Martin wrote:
> A moved-away-edit tree-conflict occurs when update changes a move
> source, so:
>
> svn mv A/B/C C2
> svn up --accept postpone .
>
> creates a moved-away-edit conflict on A/B/C. Resolving the conflict
> updates C2 by changing the NODES rows and creating workqueue items to
> modify the C2 files. These operations on C2 should be protected by a wc
> lock.
>
> The code outline is:
>
> svn_client_resolve
> svn_wc__acquire_write_lock
> svn_wc__resolve_conflicts
> svn_wc__db_update_moved_away_conflict_victim
> start txn
> update NODES
> create WQ items
> commit txn
> run WQ
> svn_wc__release_write_lock
>
> Currently svn_client_resolve acquires a lock on the parent of the
> resolve target but that may not include the move destination. When
> resolving A/B/C it acquires a lock on A/B and when recursively resolving
> A/B it acquires a lock on A; in neither case does the lock include C2.
> So the updating of C2 happens without a wc lock and that's a bug.
>
> How do we get a lock on C2? At present svn_client_resolve doesn't know
> about C2, the first time we identify C2 is inside the low level function
> svn_wc__db_update_moved_away_conflict_victim. We could acquire the lock
> at that point but we we can't release it until after the workqueue has
> been run. An extra lock on C2 won't be released automatically when we
> unlock the original lock.
>
> I see two solutions:
>
> A. svn_client_resolve continues to lock the existing tree. The lower
> level functions acquire further locks and update some data
> structure that returns the new locks. svn_client_resolve unlocks
> all the acquired locks.
>
> B. svn_client_resolve calls some new function that examines the
> conflicts in the target tree and and determines which tree to lock
> to include move destinations. The lower level function simply
> assume, and verify, that the appropriate locks are held.
>
> Solution A has the problem that acquiring additional locks may fail due
> to existing locks, and so the resolution may fail part way through. The
> lifetime of the locks is complicated as well.
>
> Solution B will lock more of tree, '.' in the example above, since it
> has to include the common parent of all the trees. However, there is no
> performance issue, it's still only one recursive lock.
>
> I prefer solution B and that's what I plan to implement.
I absolutely agree with that. Incremental locking paves the road to
madness (and deadlocks, as you observe).
-- Brane
-- Brane
--
Branko Čibej
Director of Subversion | WANdisco | www.wandisco.com
Received on 2013-02-26 15:45:11 CET