master: Fix data structure for keeping lock ranges

This commit fixes a bug in LockRanges structure,
which was caused by a missing loop terminating condition
in a function responsible for applying a lock.

Change-Id: I05cdf4e367ab9935dd1b745f25e7d2d930dba09a

src/master/locks.cc

@@ -22,6 +22,10 @@
 #include <functional>
 
 bool LockRanges::fits(const LockRange &range) const {
+	if (range.unlocking()) {
+		return true;
+	}
+
 	auto start = std::lower_bound(data_.begin(), data_.end(), range.start,
 			[](const LockRange &other, uint64_t offset) {return other.start < offset;});
 	auto end = std::upper_bound(data_.begin(), data_.end(), range.end,
@@ -34,10 +38,6 @@ bool LockRanges::fits(const LockRange &range) const {
 	for (auto it = start; it != end; ++it) {
 		const LockRange &other = *it;
 		if (range.overlaps(other)) {
-			// Range is to be removed
-			if (range.unlocking() && other.hasOwner(range.owner())) {
-				continue;
-			}
 			// Range overlaps, is not shared and owner is not the same
 			if ((!range.shared() || !other.shared()) && other.owners != range.owners) {
 				return false;
@@ -75,6 +75,11 @@ void LockRanges::insert(LockRange &range) {
 			continue;
 		}
 
+		// There are no more ranges that can overlap - break
+		if (range.end <= other.start) {
+			break;
+		}
+
 		// Ranges do not overlap, no need for collision check
 		if (!range.overlaps(other)) {
 			continue;

src/master/locks.h

@@ -227,7 +227,6 @@ class FileLocks {
 	 */
 	bool unlock(uint32_t inode, uint64_t start, uint64_t end, Owner owner);
 
-
 	/*!
 	 * Returns a list of locks from pending queue that might be available
 	 * after removing a lock from range [start, end).

src/master/locks_unittest.cc

@@ -126,8 +126,8 @@ TEST(LocksTest, Removes) {
 	EXPECT_TRUE(unlock(ranges, 10, 20, 1));
 	EXPECT_TRUE(unlock(ranges, 40, 50, 1));
 	EXPECT_TRUE(unlock(ranges, 70, 90, 1));
-	// Removing as a different user should fail
-	EXPECT_FALSE(unlock(ranges, 90, 100, 2));
+	// Removing as a different user should have no effect
+	EXPECT_TRUE(unlock(ranges, 90, 100, 2));
 
 	EXPECT_TRUE(lock_exclusive(ranges, 0, 90, 1));
 
@@ -208,3 +208,29 @@ TEST(LocksTest, PartialUnlock) {
 	 // Exclusive lock should now succeed
 	 EXPECT_TRUE(lock_exclusive(ranges, 1, 2, 1));
 }
+
+TEST(LocksTest, StressTest) {
+	LockRanges ranges;
+
+	EXPECT_TRUE(lock_exclusive(ranges, 0, 1, 7));
+
+	for(int j = 0; j < 100; j++) {
+		int i = j % 3;
+		int a = i;
+		int b = (i + 1) % 3;
+		int c = (i + 2) % 3;
+
+		EXPECT_TRUE(lock_exclusive(ranges, b, b + 1, 7));
+		EXPECT_TRUE(unlock(ranges, a, a + 1, 7));
+		EXPECT_TRUE(lock_exclusive(ranges, c, c + 1, 7));
+	}
+
+	EXPECT_TRUE(lock_exclusive(ranges, 0, 1, 6));
+
+	EXPECT_TRUE(unlock(ranges, 1, 2, 7));
+	EXPECT_FALSE(lock_exclusive(ranges, 0, 1, 7));
+
+	EXPECT_TRUE(lock_exclusive(ranges, 1, 2, 6));
+	EXPECT_TRUE(unlock(ranges, 0, 1, 6));
+	EXPECT_TRUE(lock_exclusive(ranges, 0, 1, 7));
+}