Acquiring lock by vfs.makelock() and getting lock holder (aka
"locker") information by vfs.readlock() aren't atomic action.
Therefore, failure of the former doesn't ensure success of the latter.
Before this patch, lock is unintentionally acquired, if ENOENT
causes failure of vfs.readlock() while 5 times retrying, because
lock._trylock() returns to caller silently after retrying, and
lock.lock() assumes that lock._trylock() returns successfully only if
lock is acquired.
In this case, lock symlink (or file) isn't created, even though lock
is treated as acquired in memory.
To avoid this issue, this patch makes lock._trylock() raise
LockHeld(EAGAIN) at the end of it, if lock isn't acquired while
retrying.
An empty "locker" meaning "busy for frequent lock/unlock by many
processes" might appear in an abortion message, if lock acquisition
fails. Therefore, this patch also does:
- use '%r' to increase visibility of "locker", even if it is empty
- show hint message to explain what empty "locker" means
Acquiring lock by vfs.makelock() and getting lock holder (aka
"locker") information by vfs.readlock() aren't atomic action.
Therefore, failure of the former doesn't ensure success of the latter.
Before this patch, lock is unintentionally acquired, if
self.parentlock is None (this is default value), and lock._readlock()
returns None for ENOENT at vfs.readlock(), because these None are
recognized as equal to each other.
In this case, lock symlink (or file) isn't created, even though lock
is treated as acquired in memory.
To avoid this issue, this patch retries lock acquisition immediately,
if lock._readlock() returns None "locker".
This issue will be covered by a test added in subsequent patch,
because simple emulation of ENOENT at vfs.readlock() easily causes
another issue. "raising ENOENT only at the first vfs.readlock()
invocation" is too complicated for unit test, IMHO.
Previously, the lock only contains a hostname as an attempt to detect pid
namespace difference. However, that's not enough on modern Linux - a single
hostname could have different pid namespaces.
That means if people run hg inside different PID namespaces with a same UTS
namespae, the lock would be broken - an hg proccess in pid namespace A will
think the lock having a "random" pid in pid namespace B is "dead" and remove
it.
This patch solves the above issue by appending an PID namespace identifier of
the current process to the lock prefix ("hostname"). It depends on /proc
being mounted properly. But I don't think there is a better way to get pid
namespace identifier reliably.
Post release hooks should only be called once. Setting the
list to None after usage will prevent accidental usage after
they are used.
In addition, it is easy for reference cycles to sneak into hook
functions. Clearing the hooks after usage helps prevent these
cycles.
This lets us greatly simply acquire/release cycles.
Code pattern before:
try:
lock = repo.lock()
# zillions of lines of code
finally:
lock.release()
And after:
with repo.lock():
# ...
We want to prevent locks from being inherited sometimes (e.g. when there's a
currently running transaction, which will break a lot of assumptions we're
making in here.)
Review feedback from Pierre-Yves David. This makes the overall code cleaner and
less error-prone, and makes a previously explicitly checked error state
impossible.
This is part of a series that will allow locks to be inherited by subprocesses
in limited circumstances. This patch enables the logic introduced in previous
patches.
This is part of a series that will allow locks to be inherited by subprocesses
in limited circumstances. A subprocess unlinking a lock will lead to potential
corruption from other concurrent processes.
This is part of a series that will allow locks to be inherited by subprocesses
in limited circumstances. In upcoming patches we will refer to this state.
Python 2.6 introduced the "except type as instance" syntax, replacing
the "except type, instance" syntax that came before. Python 3 dropped
support for the latter syntax. Since we no longer support Python 2.4 or
2.5, we have no need to continue supporting the "except type, instance".
This patch mass rewrites the exception syntax to be Python 2.6+ and
Python 3 compatible.
This patch was produced by running `2to3 -f except -w -n .`.
Consider a hypothetical bug in the release function that causes it to raise an
exception. Also consider the bisect command, which saves its state in a finally
clause. Saving the state requires acquiring the wlock.
If we don't unlink the lockfile when the exception is thrown, we'll try to
acquire the wlock again. We're going to try and acquire a lock again while our
old lockfile is on disk. The PID on disk is our own, and of course we're still
running, so we won't take over the lock. Hence we'll be stuck waiting for a
lock that we left behind ourselves.
To avoid this, always unlink the lockfile. This preserves the invariant that
self.held > 0 is equivalent to the lockfile existing on disk.
A message like this was sometimes shown when pushing:
remote: waiting for lock on repository foo held by 'mercurial:20858'
That could scare users, making them wonder whether the push actually succeeded.
To mitigate that fear, issue an additional "warning" such as:
got lock after 2 seconds
The return value from lock.lock.lock() was unused - instead we return the
delay.
This also adds the first test coverage for waiting for locks.
This patch makes "lock.lock.__init__()" take both vfs and lock file
path relative to vfs, instead of absolute path to lock file.
This allows lock file to be accessed via vfs.
This prevents us from having a bunch of errant worker processes all try
to release a lock if a problem occurs. (Releasing the lock more than once
is harmless; it's invoking the associated callbacks we want to avoid.)
Suppose the following scenario:
1. Process A takes the lock (e.g. on commit).
2. Process B wants to grab the lock. Since lock file exists
the exception is raised. In the catch block the testlock
function is called.
3. Process A releases the lock.
4. Process B tries to read the lock file as a part of testlock
function. This results in OSError (ENOENT) and since we're
not inside the exception handler function this is propagated
and aborts the whole operation.
To fix this we now check in testlock function whether lock file
actually exists and if not (i.e. if readlock fails) we just return.
all locks should use the explicit lock.release
mercurial.lock.lock.__del__ handles unwrapping recursive locks
localrepo.lock/wlock are still using weakref in order to keep backward
compatibiltiy to releasing locks via garbage collection
by ensuring the release on __del__
