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.
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.
wlock: do not warn for non-wait locking
We are warning about lock acquired in the wrong order because this can create
dead-lock situation. But non-wait acquisition will not block and therefore not
create a dead-lock. So we do not need to wait in such case.
develwarn: include call site in the simple message version
Just displaying the warning makes it quite hard to recognise the guilty code
quickly and using --traceback for all calls is not very convenient. So we
include the call site with all simple message to help developer to recognise
errors sources.
develwarn: handle the end of line inside the function itself
The traceback version should not have a end of line at all. The non-traceback
version will requires the same things soon.
develwarn: refactor the developer warning logic
The logic is currently duplicated and we plan to make it a bit smarter. So we
move it into a function first to make the update more robust and simple.
lock: update the docstring with order information
Lock must be acquired in a specific order to avoid dead-lock. This was
documented on the wiki, but having this information in the docstring is also
useful.
wlock: reword the devel warning
We change the wording of the developer warning:
- "lock" taken before "wlock"
+ "wlock" acquired after "lock"
The goals here are to:
- Put the "subject" as the first word,
- use "acquired" instead of "taken" since it seems more accurate.
wlock: only issue devel warning when actually acquiring the lock
Before this change, any call to 'wlock' after we acquired a 'lock' was issuing a
warning. This is wrong as the 'wlock' have been properly acquired before the
'lock' in a previous call.
We move the warning code to only issue such warnings when we are acquiring the
'wlock' -after- acquiring 'lock'. This is the expected behavior of this warning
from the start.
bundle2: flush output in a part in all cases
We want to preserve output even when the unbundling fails (eg: hook output). So
we must make sure that everything we have is flushed into the reply bundle.
(This is related to
issue4594)