##// END OF EJS Templates
lock: fix race in lock-breaking code...
lock: fix race in lock-breaking code With low frequency, I see hg pulls fail with output like: abort: no such file or directory: .hg/store/lock I think what happens is, in lock.py, in: def _testlock(self, locker): if not self._lockshouldbebroken(locker): return locker # if locker dead, break lock. must do this with another lock # held, or can race and break valid lock. try: with lock(self.vfs, self.f + b'.break', timeout=0): self.vfs.unlink(self.f) except error.LockError: return locker if a lock is breakable on disk, and two hg processes concurrently get to the "if locker dead" comment, a possible interleaving is: process1 finishes executing the function and then process2 finishes executing the function. If that happens, process2 will either get ENOENT in self.vfs.unlink (resulting in the spurious failure above), or break a valid lock and potentially cause repository corruption. The fix is simple enough: make sure the lock is breakable _inside_ the critical section, because only then can we know that no other process can invalidate our knowledge on the lock on disk. I don't think there are tests for this. I've tested this manually with: diff --git a/mercurial/lock.py b/mercurial/lock.py --- a/mercurial/lock.py +++ b/mercurial/lock.py @@ -351,6 +351,8 @@ class lock(object): if not self._lockshouldbebroken(locker): return locker + import random + time.sleep(1. + random.random()) # if locker dead, break lock. must do this with another lock # held, or can race and break valid lock. try: @@ -358,6 +360,7 @@ class lock(object): self.vfs.unlink(self.f) except error.LockError: return locker + time.sleep(1) def testlock(self): """return id of locker if lock is valid, else None. and I see this change of behavior before/after this commit: $ $hg init repo $ cd repo $ ln -s $HOSTNAME/effffffc:987654321 .hg/wlock $ touch a $ $hg commit -Am_ & $hg commit -Am _; wait -abort: No such file or directory: '/tmp/repo/.hg/wlock' adding a +warning: ignoring unknown working parent 679a8959a8ca! +nothing changed Differential Revision: https://phab.mercurial-scm.org/D7199

File last commit:

r44085:53607fd3 stable
r44108:039fbd14 default
Show More
osutilbuild.py
108 lines | 2.5 KiB | text/x-python | PythonLexer
from __future__ import absolute_import
import cffi
ffi = cffi.FFI()
ffi.set_source(
b"mercurial.cffi._osutil",
"""
#include <sys/attr.h>
#include <sys/vnode.h>
#include <unistd.h>
#include <fcntl.h>
#include <time.h>
typedef struct val_attrs {
uint32_t length;
attribute_set_t returned;
attrreference_t name_info;
fsobj_type_t obj_type;
struct timespec mtime;
uint32_t accessmask;
off_t datalength;
} __attribute__((aligned(4), packed)) val_attrs_t;
""",
include_dirs=[b'mercurial'],
)
ffi.cdef(
'''
typedef uint32_t attrgroup_t;
typedef struct attrlist {
uint16_t bitmapcount; /* number of attr. bit sets in list */
uint16_t reserved; /* (to maintain 4-byte alignment) */
attrgroup_t commonattr; /* common attribute group */
attrgroup_t volattr; /* volume attribute group */
attrgroup_t dirattr; /* directory attribute group */
attrgroup_t fileattr; /* file attribute group */
attrgroup_t forkattr; /* fork attribute group */
...;
};
typedef struct attribute_set {
...;
} attribute_set_t;
typedef struct attrreference {
int attr_dataoffset;
int attr_length;
...;
} attrreference_t;
typedef int ... off_t;
typedef struct val_attrs {
uint32_t length;
attribute_set_t returned;
attrreference_t name_info;
uint32_t obj_type;
struct timespec mtime;
uint32_t accessmask;
off_t datalength;
...;
} val_attrs_t;
/* the exact layout of the above struct will be figured out during build time */
typedef int ... time_t;
typedef struct timespec {
time_t tv_sec;
...;
};
int getattrlist(const char* path, struct attrlist * attrList, void * attrBuf,
size_t attrBufSize, unsigned int options);
int getattrlistbulk(int dirfd, struct attrlist * attrList, void * attrBuf,
size_t attrBufSize, uint64_t options);
#define ATTR_BIT_MAP_COUNT ...
#define ATTR_CMN_NAME ...
#define ATTR_CMN_OBJTYPE ...
#define ATTR_CMN_MODTIME ...
#define ATTR_CMN_ACCESSMASK ...
#define ATTR_CMN_ERROR ...
#define ATTR_CMN_RETURNED_ATTRS ...
#define ATTR_FILE_DATALENGTH ...
#define VREG ...
#define VDIR ...
#define VLNK ...
#define VBLK ...
#define VCHR ...
#define VFIFO ...
#define VSOCK ...
#define S_IFMT ...
int open(const char *path, int oflag, int perm);
int close(int);
#define O_RDONLY ...
'''
)
if __name__ == '__main__':
ffi.compile()