Release locks on destruction
Project: http://git-wip-us.apache.org/repos/asf/lucy/repo Commit: http://git-wip-us.apache.org/repos/asf/lucy/commit/d23b560d Tree: http://git-wip-us.apache.org/repos/asf/lucy/tree/d23b560d Diff: http://git-wip-us.apache.org/repos/asf/lucy/diff/d23b560d Branch: refs/heads/master Commit: d23b560dc34d1c1cbc731fdc745b4e72324d8717 Parents: 35388cd Author: Nick Wellnhofer <wellnho...@aevum.de> Authored: Sun Feb 19 14:09:02 2017 +0100 Committer: Nick Wellnhofer <wellnho...@aevum.de> Committed: Mon Feb 20 16:51:30 2017 +0100 ---------------------------------------------------------------------- core/Lucy/Index/BackgroundMerger.c | 2 -- core/Lucy/Index/FilePurger.c | 7 +---- core/Lucy/Index/IndexReader.c | 6 +--- core/Lucy/Index/Indexer.c | 2 -- core/Lucy/Index/PolyReader.c | 1 - core/Lucy/Store/Lock.c | 2 +- core/Lucy/Store/Lock.cfh | 2 +- test/Lucy/Test/Store/TestLock.c | 51 ++++++++++++++++----------------- 8 files changed, 29 insertions(+), 44 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/core/Lucy/Index/BackgroundMerger.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/BackgroundMerger.c b/core/Lucy/Index/BackgroundMerger.c index be69d90..d326140 100644 --- a/core/Lucy/Index/BackgroundMerger.c +++ b/core/Lucy/Index/BackgroundMerger.c @@ -559,7 +559,6 @@ static void S_release_write_lock(BackgroundMerger *self) { BackgroundMergerIVARS *const ivars = BGMerger_IVARS(self); if (ivars->write_lock) { - Lock_Release(ivars->write_lock); DECREF(ivars->write_lock); ivars->write_lock = NULL; } @@ -569,7 +568,6 @@ static void S_release_merge_lock(BackgroundMerger *self) { BackgroundMergerIVARS *const ivars = BGMerger_IVARS(self); if (ivars->merge_lock) { - Lock_Release(ivars->merge_lock); DECREF(ivars->merge_lock); ivars->merge_lock = NULL; } http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/core/Lucy/Index/FilePurger.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/FilePurger.c b/core/Lucy/Index/FilePurger.c index 5795bb0..211f0ea 100644 --- a/core/Lucy/Index/FilePurger.c +++ b/core/Lucy/Index/FilePurger.c @@ -123,17 +123,12 @@ FilePurger_Purge_Snapshots_IMP(FilePurger *self, Snapshot *current) { } } - // Release snapshot locks. - for (size_t i = 0, max = Vec_Get_Size(locks); i < max; i++) { - Lock_Release((Lock*)Vec_Fetch(locks, i)); - } - DECREF(iter); DECREF(failures); DECREF(purged); DECREF(spared); DECREF(snapshots); - DECREF(locks); + DECREF(locks); // Will release locks. } void http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/core/Lucy/Index/IndexReader.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/IndexReader.c b/core/Lucy/Index/IndexReader.c index dabb3be..ce3a205 100644 --- a/core/Lucy/Index/IndexReader.c +++ b/core/Lucy/Index/IndexReader.c @@ -81,7 +81,6 @@ IxReader_Close_IMP(IndexReader *self) { Hash_Clear(ivars->components); } if (ivars->snapshot_lock) { - Lock_Release(ivars->snapshot_lock); DECREF(ivars->snapshot_lock); ivars->snapshot_lock = NULL; } @@ -91,10 +90,7 @@ void IxReader_Destroy_IMP(IndexReader *self) { IndexReaderIVARS *const ivars = IxReader_IVARS(self); DECREF(ivars->components); - if (ivars->snapshot_lock) { - Lock_Release(ivars->snapshot_lock); - DECREF(ivars->snapshot_lock); - } + DECREF(ivars->snapshot_lock); DECREF(ivars->manager); SUPER_DESTROY(self, INDEXREADER); } http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/core/Lucy/Index/Indexer.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/Indexer.c b/core/Lucy/Index/Indexer.c index 9190b93..fc6afec 100644 --- a/core/Lucy/Index/Indexer.c +++ b/core/Lucy/Index/Indexer.c @@ -587,7 +587,6 @@ static void S_release_write_lock(Indexer *self) { IndexerIVARS *const ivars = Indexer_IVARS(self); if (ivars->write_lock) { - Lock_Release(ivars->write_lock); DECREF(ivars->write_lock); ivars->write_lock = NULL; } @@ -597,7 +596,6 @@ static void S_release_merge_lock(Indexer *self) { IndexerIVARS *const ivars = Indexer_IVARS(self); if (ivars->merge_lock) { - Lock_Release(ivars->merge_lock); DECREF(ivars->merge_lock); ivars->merge_lock = NULL; } http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/core/Lucy/Index/PolyReader.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Index/PolyReader.c b/core/Lucy/Index/PolyReader.c index 6b9e943..572cc2b 100644 --- a/core/Lucy/Index/PolyReader.c +++ b/core/Lucy/Index/PolyReader.c @@ -473,7 +473,6 @@ static void S_release_snapshot_lock(PolyReader *self) { PolyReaderIVARS *const ivars = PolyReader_IVARS(self); if (ivars->snapshot_lock) { - Lock_Release(ivars->snapshot_lock); DECREF(ivars->snapshot_lock); ivars->snapshot_lock = NULL; } http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/core/Lucy/Store/Lock.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Store/Lock.c b/core/Lucy/Store/Lock.c index 5cddcd9..c4542a6 100644 --- a/core/Lucy/Store/Lock.c +++ b/core/Lucy/Store/Lock.c @@ -454,7 +454,7 @@ S_maybe_delete_file(LockFileLockIVARS *ivars, String *path, void LFLock_Destroy_IMP(LockFileLock *self) { LockFileLockIVARS *const ivars = LFLock_IVARS(self); - DECREF(ivars->shared_lock_path); + if (ivars->state != LFLOCK_STATE_UNLOCKED) { LFLock_Release(self); } DECREF(ivars->link_path); SUPER_DESTROY(self, LOCKFILELOCK); } http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/core/Lucy/Store/Lock.cfh ---------------------------------------------------------------------- diff --git a/core/Lucy/Store/Lock.cfh b/core/Lucy/Store/Lock.cfh index 30940b4..6a4b719 100644 --- a/core/Lucy/Store/Lock.cfh +++ b/core/Lucy/Store/Lock.cfh @@ -91,7 +91,7 @@ abstract class Lucy::Store::Lock inherits Clownfish::Obj { public abstract bool Request_Exclusive(Lock *self); - /** Release the lock. + /** Release the lock. Locks are always released by the destructor. */ public abstract void Release(Lock *self); http://git-wip-us.apache.org/repos/asf/lucy/blob/d23b560d/test/Lucy/Test/Store/TestLock.c ---------------------------------------------------------------------- diff --git a/test/Lucy/Test/Store/TestLock.c b/test/Lucy/Test/Store/TestLock.c index f606e7b..b52f366 100644 --- a/test/Lucy/Test/Store/TestLock.c +++ b/test/Lucy/Test/Store/TestLock.c @@ -71,9 +71,10 @@ test_exclusive_only(TestBatchRunner *runner, Lock *lock1, Lock *lock2, "Request_Exclusive (only) succeeds after Release %s", tag); TEST_FALSE(runner, Lock_Request_Exclusive(lock1), "Request_Exclusive (only) fails if lock2 is locked %s", tag); - Lock_Release(lock2); - DECREF(lock2); + + TEST_TRUE(runner, Lock_Request_Exclusive(lock1), + "Request_Exclusive (only) succeeds after Destroy %s", tag); DECREF(lock1); } @@ -105,27 +106,12 @@ test_lock(TestBatchRunner *runner, Lock *lock1, Lock *lock2, Lock *lock3, TEST_TRUE(runner, CERTIFY(Err_get_error(), LOCKERR), "Request_Exclusive after Request_Exclusive sets LockErr %s", tag); - Lock_Release(lock1); - - TEST_TRUE(runner, Lock_Request_Exclusive(lock3), - "Request_Exclusive succeeds after Release %s", tag); - Lock_Release(lock3); - - DECREF(lock3); - DECREF(lock2); DECREF(lock1); -} -static void -test_change_pid(TestBatchRunner *runner, Folder *folder, String *path, - const char *tag) { - Hash *hash = (Hash*)Json_slurp_json(folder, path); - TEST_TRUE(runner, CERTIFY(hash, HASH), "Lock file %s exists %s", - Str_Get_Ptr8(path), tag); - Hash_Store(hash, SSTR_WRAP_C("pid"), (Obj*)Str_newf("10000000")); - Folder_Delete(folder, path); - Json_spew_json((Obj*)hash, folder, path); - DECREF(hash); + TEST_TRUE(runner, Lock_Request_Shared(lock2), + "Request_Shared succeeds after Destroy %s", tag); + DECREF(lock2); + DECREF(lock3); } static void @@ -136,11 +122,19 @@ test_stale(TestBatchRunner *runner, Folder *folder, const char *tag) { LockFileLock *lock1 = LFLock_new(folder, name, host1, 0, 100, false); LockFileLock *lock2 = LFLock_new(folder, name, host2, 0, 100, false); LockFileLock *tmp; + Hash *hash; tmp = LFLock_new(folder, name, host1, 0, 100, false); LFLock_Request_Exclusive(tmp); + String *ex_path = SSTR_WRAP_C("locks/test.lock"); + hash = (Hash*)Json_slurp_json(folder, ex_path); + TEST_TRUE(runner, CERTIFY(hash, HASH), "Lock file %s exists %s", + Str_Get_Ptr8(ex_path), tag); DECREF(tmp); - test_change_pid(runner, folder, SSTR_WRAP_C("locks/test.lock"), tag); + // Write lock file with different pid. + Hash_Store(hash, SSTR_WRAP_C("pid"), (Obj*)Str_newf("10000000")); + Json_spew_json((Obj*)hash, folder, ex_path); + DECREF(hash); TEST_FALSE(runner, LFLock_Request_Exclusive(lock2), "Lock_Exclusive fails with other host's exclusive lock %s", tag); @@ -150,8 +144,15 @@ test_stale(TestBatchRunner *runner, Folder *folder, const char *tag) { tmp = LFLock_new(folder, name, host1, 0, 100, false); LFLock_Request_Shared(tmp); + String *sh_path = SSTR_WRAP_C("locks/test-1.lock"); + hash = (Hash*)Json_slurp_json(folder, sh_path); + TEST_TRUE(runner, CERTIFY(hash, HASH), "Lock file %s exists %s", + Str_Get_Ptr8(sh_path), tag); DECREF(tmp); - test_change_pid(runner, folder, SSTR_WRAP_C("locks/test-1.lock"), tag); + // Write lock file with different pid. + Hash_Store(hash, SSTR_WRAP_C("pid"), (Obj*)Str_newf("10000000")); + Json_spew_json((Obj*)hash, folder, SSTR_WRAP_C("locks/test-98765.lock")); + DECREF(hash); TEST_FALSE(runner, LFLock_Request_Exclusive(lock2), "Lock_Exclusive fails with other host's shared lock %s", tag); TEST_TRUE(runner, LFLock_Request_Exclusive(lock1), @@ -175,8 +176,6 @@ test_Obtain(TestBatchRunner *runner, Lock *lock1, Lock *lock2, "Obtain_Exclusive succeeds %s", tag); TEST_FALSE(runner, Lock_Obtain_Shared(lock2), "Obtain_Shared after Obtain_Exclusive fails %s", tag); - Lock_Release(lock1); - DECREF(lock2); DECREF(lock1); } @@ -291,7 +290,7 @@ test_lf_lock(TestBatchRunner *runner) { void TestLFLock_Run_IMP(TestLockFileLock *self, TestBatchRunner *runner) { - TestBatchRunner_Plan(runner, (TestBatch*)self, 80); + TestBatchRunner_Plan(runner, (TestBatch*)self, 82); test_lf_lock(runner); }