Bug 4596 - during bundle2 processing, parts (pushkey) may acquires 'wlock' after the lock
Summary: during bundle2 processing, parts (pushkey) may acquires 'wlock' after the lock
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: 3.2
Hardware: PC Linux
: normal bug
Assignee: Bugzilla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-12 15:54 UTC by Pierre-Yves David
Modified: 2015-04-28 01:00 UTC (History)
2 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Pierre-Yves David 2015-04-12 15:54 UTC

    
Comment 1 Pierre-Yves David 2015-04-12 15:56 UTC
pushing bookmark will acquires the 'wlock' while the 'lock' have been acquired around the whole unbundling.

This is "wrong" according our locking order rules and could lead to deadlock.

We probably need to acquires the wlock for unbundling, this makes me sad :-(
Comment 2 HG Bot 2015-04-16 18:46 UTC
Fixed by http://selenic.com/repo/hg/rev/dc4daf028f9c
Pierre-Yves David <pierre-yves.david@fb.com>
run-test: enable the devel warning during tests

This should help us to catch new locking order issues as soon as possible.

There are two harmless test updates (from the config change). Moreover, some
bundle2 tests are displaying warning for a legitimate reason. The use of pushkey
during the unbundle process may requires the 'wlock' after 'lock' (around the
whole unbundle process was taken). This is non-trivial to fix, so I better have
the check on, with the warning in the test than the check off. See issue4596 for
details.

(please test the fix)
Comment 3 HG Bot 2015-04-16 18:46 UTC
Fixed by http://selenic.com/repo/hg/rev/5640efd1b160
Pierre-Yves David <pierre-yves.david@fb.com>
unbundle: acquire 'wlock' when processing bundle2 (BC) (issue4596)

A bundle2 may contain bookmark updates (or other extension content) that
requires the 'wlock' to be written. As 'wlock' must be acquired before 'lock',
we must stay on the side of caution and use both in all case to ensure their
ordering.

(please test the fix)
Comment 4 HG Bot 2015-04-16 18:46 UTC
Fixed by http://selenic.com/repo/hg/rev/5dc5cd7abbf5
Pierre-Yves David <pierre-yves.david@fb.com>
push: acquire local 'wlock' if "pushback" is expected (BC) (issue4596)

If the client allows "pushback", the bundle2 served back by the server may
contains parts that will write to the repository. Such parts may require the
'wlock' (eg: bookmark) so we acquire it in advance to make sure it got acquired
before the 'lock'.

(please test the fix)
Comment 5 HG Bot 2015-04-20 13:15 UTC
Fixed by http://selenic.com/repo/hg/rev/57f1dbc99631
Pierre-Yves David <pierre-yves.david@fb.com>
afterlock: add the callback to the top level lock (issue4608)

If 'wlock' is taken, we should add 'afterlock' callback to the 'wlock' instead.
Otherwise, running post transaction hook after 'lock' is release but 'wlock' is
still taken lead to a deadlock (eg: 'hg update' during a hook).

This situation is much more common since: 5dc5cd7abbf5

  push: acquire local 'wlock' if "pushback" is expected (BC) (issue4596)

(please test the fix)
Comment 6 Bugzilla 2015-04-28 01:00 UTC
Bug was set to TESTING for 7 days, resolving