Always check for stale locks in Is_Locked Note that checking for stale locks isn't necessary when requesting shared locks.
Project: http://git-wip-us.apache.org/repos/asf/lucy/repo Commit: http://git-wip-us.apache.org/repos/asf/lucy/commit/1a8e02df Tree: http://git-wip-us.apache.org/repos/asf/lucy/tree/1a8e02df Diff: http://git-wip-us.apache.org/repos/asf/lucy/diff/1a8e02df Branch: refs/heads/master Commit: 1a8e02df0a0257e4ba70de4a41504507b0ba8a47 Parents: bb83d0d Author: Nick Wellnhofer <wellnho...@aevum.de> Authored: Thu Feb 16 16:47:03 2017 +0100 Committer: Nick Wellnhofer <wellnho...@aevum.de> Committed: Mon Feb 20 16:26:21 2017 +0100 ---------------------------------------------------------------------- core/Lucy/Index/BackgroundMerger.c | 2 -- core/Lucy/Index/FilePurger.c | 5 ---- core/Lucy/Index/Indexer.c | 1 - core/Lucy/Index/PolyReader.c | 2 -- core/Lucy/Store/Lock.c | 48 +++++++++------------------------ core/Lucy/Store/Lock.cfh | 10 ------- go/build.go | 1 - go/lucy/store.go | 7 ----- go/lucy/store_test.go | 5 ---- perl/t/106-locking.t | 1 - perl/t/110-shared_lock.t | 5 ++-- 11 files changed, 14 insertions(+), 73 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/BackgroundMerger.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/BackgroundMerger.c b/core/Lucy/Index/BackgroundMerger.c index 661b9a6..e9a1ae8 100644 --- a/core/Lucy/Index/BackgroundMerger.c +++ b/core/Lucy/Index/BackgroundMerger.c @@ -529,7 +529,6 @@ static void S_obtain_write_lock(BackgroundMerger *self) { BackgroundMergerIVARS *const ivars = BGMerger_IVARS(self); Lock *write_lock = IxManager_Make_Write_Lock(ivars->manager); - Lock_Clear_Stale(write_lock); if (Lock_Obtain_Exclusive(write_lock)) { // Only assign if successful, otherwise DESTROY unlocks -- bad! ivars->write_lock = write_lock; @@ -543,7 +542,6 @@ static void S_obtain_merge_lock(BackgroundMerger *self) { BackgroundMergerIVARS *const ivars = BGMerger_IVARS(self); Lock *merge_lock = IxManager_Make_Merge_Lock(ivars->manager); - Lock_Clear_Stale(merge_lock); if (Lock_Obtain_Exclusive(merge_lock)) { // Only assign if successful, same rationale as above. ivars->merge_lock = merge_lock; http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/FilePurger.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/FilePurger.c b/core/Lucy/Index/FilePurger.c index 3980c8f..161f030 100644 --- a/core/Lucy/Index/FilePurger.c +++ b/core/Lucy/Index/FilePurger.c @@ -78,7 +78,6 @@ FilePurger_Purge_Snapshots_IMP(FilePurger *self, Snapshot *current) { Lock *deletion_lock = IxManager_Make_Deletion_Lock(ivars->manager); // Obtain deletion lock, purge files, release deletion lock. - Lock_Clear_Stale(deletion_lock); if (Lock_Obtain_Exclusive(deletion_lock)) { Folder *folder = ivars->folder; Hash *failures = Hash_new(16); @@ -147,7 +146,6 @@ FilePurger_Purge_Aborted_Merge_IMP(FilePurger *self) { IndexManager *manager = ivars->manager; Lock *merge_lock = IxManager_Make_Merge_Lock(manager); - Lock_Clear_Stale(merge_lock); if (!Lock_Is_Locked_Exclusive(merge_lock)) { Hash *merge_data = IxManager_Read_Merge_Data(manager); Obj *cutoff = merge_data @@ -213,9 +211,6 @@ S_discover_unused(FilePurger *self, Snapshot *current, Hash *spared, // DON'T obtain the lock -- only see whether another // entity holds a lock on the snapshot file. - if (lock) { - Lock_Clear_Stale(lock); - } if (lock && Lock_Is_Locked(lock)) { // The snapshot file is locked, which means someone's using // that version of the index -- protect all of its entries. http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/Indexer.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/Indexer.c b/core/Lucy/Index/Indexer.c index 97763e3..2381b26 100644 --- a/core/Lucy/Index/Indexer.c +++ b/core/Lucy/Index/Indexer.c @@ -97,7 +97,6 @@ Indexer_init(Indexer *self, Schema *schema, Obj *index, // Get a write lock for this folder. Lock *write_lock = IxManager_Make_Write_Lock(ivars->manager); - Lock_Clear_Stale(write_lock); if (Lock_Obtain_Exclusive(write_lock)) { // Only assign if successful, otherwise DESTROY unlocks -- bad! ivars->write_lock = write_lock; http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Index/PolyReader.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/PolyReader.c b/core/Lucy/Index/PolyReader.c index d4166c6..ea5e35f 100644 --- a/core/Lucy/Index/PolyReader.c +++ b/core/Lucy/Index/PolyReader.c @@ -470,7 +470,6 @@ static bool S_obtain_deletion_lock(PolyReader *self) { PolyReaderIVARS *const ivars = PolyReader_IVARS(self); ivars->deletion_lock = IxManager_Make_Deletion_Lock(ivars->manager); - Lock_Clear_Stale(ivars->deletion_lock); if (!Lock_Obtain_Exclusive(ivars->deletion_lock)) { DECREF(ivars->deletion_lock); ivars->deletion_lock = NULL; @@ -485,7 +484,6 @@ S_obtain_read_lock(PolyReader *self, String *snapshot_file_name) { ivars->read_lock = IxManager_Make_Snapshot_Read_Lock(ivars->manager, snapshot_file_name); - Lock_Clear_Stale(ivars->read_lock); if (!Lock_Obtain_Shared(ivars->read_lock)) { DECREF(ivars->read_lock); ivars->read_lock = NULL; http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Store/Lock.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Store/Lock.c b/core/Lucy/Store/Lock.c index 0eb42da..6d39362 100644 --- a/core/Lucy/Store/Lock.c +++ b/core/Lucy/Store/Lock.c @@ -341,7 +341,8 @@ bool LFLock_Is_Locked_Exclusive_IMP(LockFileLock *self) { LockFileLockIVARS *const ivars = LFLock_IVARS(self); - return Folder_Exists(ivars->folder, ivars->lock_path); + return Folder_Exists(ivars->folder, ivars->lock_path) + && !S_maybe_delete_file(ivars, ivars->lock_path, false, true); } bool @@ -349,7 +350,9 @@ LFLock_Is_Locked_IMP(LockFileLock *self) { LockFileLockIVARS *const ivars = LFLock_IVARS(self); // Check for exclusive lock. - if (Folder_Exists(ivars->folder, ivars->lock_path)) { + if (Folder_Exists(ivars->folder, ivars->lock_path) + && !S_maybe_delete_file(ivars, ivars->lock_path, false, true) + ) { return true; } @@ -360,51 +363,24 @@ LFLock_Is_Locked_IMP(LockFileLock *self) { return false; } + bool locked = false; DirHandle *dh = Folder_Open_Dir(ivars->folder, lock_dir_name); if (!dh) { RETHROW(INCREF(Err_get_error())); } while (DH_Next(dh)) { String *entry = DH_Get_Entry(dh); if (S_is_shared_lock_file(ivars, entry)) { - DECREF(entry); - DECREF(dh); - return true; + String *candidate = Str_newf("%o/%o", lock_dir_name, entry); + if (!S_maybe_delete_file(ivars, candidate, false, true)) { + locked = true; + } + DECREF(candidate); } DECREF(entry); } DECREF(dh); - return false; -} - -void -LFLock_Clear_Stale_IMP(LockFileLock *self) { - LockFileLockIVARS *const ivars = LFLock_IVARS(self); - - if (ivars->shared_lock_path) { - String *lock_dir_name = SSTR_WRAP_C("locks"); - if (!Folder_Find_Folder(ivars->folder, lock_dir_name)) { - return; - } - - DirHandle *dh = Folder_Open_Dir(ivars->folder, lock_dir_name); - if (!dh) { RETHROW(INCREF(Err_get_error())); } - - while (DH_Next(dh)) { - String *entry = DH_Get_Entry(dh); - if (S_is_shared_lock_file(ivars, entry)) { - String *candidate = Str_newf("%o/%o", lock_dir_name, entry); - S_maybe_delete_file(ivars, candidate, false, true); - DECREF(candidate); - } - DECREF(entry); - } - - DECREF(dh); - } - else { - S_maybe_delete_file(ivars, ivars->lock_path, false, true); - } + return locked; } static bool http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/core/Lucy/Store/Lock.cfh ---------------------------------------------------------------------- diff --git a/core/Lucy/Store/Lock.cfh b/core/Lucy/Store/Lock.cfh index 3f93dc0..598c777 100644 --- a/core/Lucy/Store/Lock.cfh +++ b/core/Lucy/Store/Lock.cfh @@ -112,13 +112,6 @@ public abstract class Lucy::Store::Lock inherits Clownfish::Obj { public abstract bool Is_Locked_Exclusive(Lock *self); - /** Release all locks that meet the following three conditions: the lock - * name matches, the host id matches, and the process id that the lock - * was created under no longer identifies an active process. - */ - public abstract void - Clear_Stale(Lock *self); - String* Get_Name(Lock *self); @@ -159,9 +152,6 @@ class Lucy::Store::LockFileLock nickname LFLock public bool Is_Locked_Exclusive(LockFileLock *self); - public void - Clear_Stale(LockFileLock *self); - String* Get_Lock_Path(LockFileLock *self); http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/go/build.go ---------------------------------------------------------------------- diff --git a/go/build.go b/go/build.go index 16c31f2..d64cbd2 100644 --- a/go/build.go +++ b/go/build.go @@ -458,7 +458,6 @@ func specClasses(parcel *cfc.Parcel) { lockBinding.SpecMethod("Obtain_Shared", "ObtainShared() error") lockBinding.SpecMethod("Obtain_Exclusive", "ObtainExclusive() error") lockBinding.SpecMethod("Release", "Release() error") - lockBinding.SpecMethod("Clear_Stale", "ClearStale() error") lockBinding.Register() cfWriterBinding := cfc.NewGoClass(parcel, "Lucy::Store::CompoundFileWriter") http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/go/lucy/store.go ---------------------------------------------------------------------- diff --git a/go/lucy/store.go b/go/lucy/store.go index 9b9d8c3..7f00a94 100644 --- a/go/lucy/store.go +++ b/go/lucy/store.go @@ -766,13 +766,6 @@ func (lock *LockIMP) Release() error { } -func (lock *LockIMP) ClearStale() error { - return clownfish.TrapErr(func() { - self := (*C.lucy_Lock)(clownfish.Unwrap(lock, "lock")) - C.LUCY_Lock_Clear_Stale(self) - }) -} - func OpenCompoundFileReader(folder Folder) (reader CompoundFileReader, err error) { err = clownfish.TrapErr(func() { folderC := (*C.lucy_Folder)(clownfish.Unwrap(folder, "Folder")) http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/go/lucy/store_test.go ---------------------------------------------------------------------- diff --git a/go/lucy/store_test.go b/go/lucy/store_test.go index 680b9b2..ec51244 100644 --- a/go/lucy/store_test.go +++ b/go/lucy/store_test.go @@ -710,11 +710,6 @@ func TestLockFileLockAll(t *testing.T) { t.Errorf("Obtain: %v", err) } lock.Release() - - err = lock.ClearStale() - if err != nil { - t.Errorf("Nothing for ClearStale to do, but should still suceed: %v", err) - } } func TestLockFactoryAll(t *testing.T) { http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/perl/t/106-locking.t ---------------------------------------------------------------------- diff --git a/perl/t/106-locking.t b/perl/t/106-locking.t index 3a1670c..15c6a3a 100644 --- a/perl/t/106-locking.t +++ b/perl/t/106-locking.t @@ -50,7 +50,6 @@ Dead_locks_are_removed: { exclusive_only => 1, @_ ); - $lock->clear_stale; $lock->obtain_exclusive() or die "no dice"; return $lock; } http://git-wip-us.apache.org/repos/asf/lucy/blob/1a8e02df/perl/t/110-shared_lock.t ---------------------------------------------------------------------- diff --git a/perl/t/110-shared_lock.t b/perl/t/110-shared_lock.t index 5281c9f..77c6862 100644 --- a/perl/t/110-shared_lock.t +++ b/perl/t/110-shared_lock.t @@ -74,9 +74,8 @@ $lock->release; $another_lock->release; $ya_lock->release; -ok( $lock->is_locked(), "failed to release a lock with a different pid" ); -$lock->clear_stale; -ok( !$lock->is_locked(), "clear_stale" ); +ok( $lock->get_lock_path, "failed to release a lock with a different pid" ); +ok( !$lock->is_locked(), "is_locked clears stale locks" ); ok( $lock->obtain_shared(), "got lock again" ); ok( $lock->is_locked(), "it's locked" );