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
