From 17d203aa38b46b9aeb35dafbcb772b9275aa9b08 Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan@randomdan.homeip.net>
Date: Fri, 29 Dec 2017 20:52:48 +0000
Subject: Fix locking issues around BG writes and add covering unit tests

---
 netfs/fuse/fuseFiles.cpp     | 48 +++++++++++++++++++++++---------------------
 netfs/unittests/testCore.cpp | 46 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 23 deletions(-)

diff --git a/netfs/fuse/fuseFiles.cpp b/netfs/fuse/fuseFiles.cpp
index ed0e881..b587809 100644
--- a/netfs/fuse/fuseFiles.cpp
+++ b/netfs/fuse/fuseFiles.cpp
@@ -2,6 +2,7 @@
 #include "fuseApp.impl.h"
 #include "lockHelpers.h"
 #include <entCache.h>
+#include <mutex>
 
 namespace NetFS {
 FuseApp::OpenFile::OpenFile(FilePrx r, const std::string & p, int f) :
@@ -130,31 +131,32 @@ FuseApp::write(const char *, const char * buf, size_t s, off_t o, struct fuse_fi
 		if (fcr->Async) {
 			const auto key = OpenFile::range(o, s);
 			while (true) {
-				ScopeLock(of->_lock) {
-					// Acquire operations to wait for
-					std::vector<Ice::AsyncResultPtr> overlap;
-					auto R = of->bg.equal_range(key);
-					if (R.first == R.second) {
-						// Begin write and store operation
-						auto r = remote->begin_write(o, s, Buffer(buf, buf + s), [of,key](const Ice::AsyncResultPtr &) {
-							ScopeLock(of->_lock) {
-								of->bg.erase(key);
-							}
-						});
-						of->bg.insert({key, r});
-						return s;
-					}
-					else {
-						// Wait for them whilst unlocked
-						of->_lock.unlock();
-						overlap.reserve(std::distance(R.first, R.second));
-						for (auto i = R.first; i != R.second; i++) {
-							overlap.push_back(i->second);
-						}
-						for (const auto & r : overlap) {
-							r->waitForCompleted();
+				std::unique_lock<decltype(of->_lock)> _l(of->_lock);
+				// Acquire operations to wait for
+				std::vector<Ice::AsyncResultPtr> overlap;
+				auto R = of->bg.equal_range(key);
+				if (R.first == R.second) {
+					// Begin write and store operation
+					auto r = remote->begin_write(o, s, Buffer(buf, buf + s), [of,key](const Ice::AsyncResultPtr &) {
+						ScopeLock(of->_lock) {
+							of->bg.erase(key);
 						}
+					});
+					of->bg.insert({key, r});
+					break;
+				}
+				else {
+					// Wait for them whilst unlocked
+					_l.release()->unlock();
+					overlap.reserve(std::distance(R.first, R.second));
+					for (auto i = R.first; i != R.second; i++) {
+						overlap.push_back(i->second);
+					}
+					for (const auto & r : overlap) {
+						r->waitForCompleted();
 					}
+					// Cause this thread to yield so the callback can acquire _lock
+					usleep(0);
 				}
 			}
 		}
diff --git a/netfs/unittests/testCore.cpp b/netfs/unittests/testCore.cpp
index acd3f91..3d23870 100644
--- a/netfs/unittests/testCore.cpp
+++ b/netfs/unittests/testCore.cpp
@@ -286,6 +286,52 @@ BOOST_AUTO_TEST_CASE( files )
 	BOOST_REQUIRE_EQUAL(fuse->unlink("/test3"), -ENOENT);
 }
 
+BOOST_AUTO_TEST_CASE( bgwriteOverlapped, * boost::unit_test::timeout(2) )
+{
+	struct fuse_file_info fi;
+	memset(&fi, 0, sizeof(fi));
+	struct stat st;
+	memset(&st, 0, sizeof(st));
+	fi.flags = O_RDWR;
+	auto s = sizeof(int32_t);
+	auto N = 20;
+	BOOST_REQUIRE_EQUAL(fuse->create("/test", 0600, &fi), 0);
+	for (int32_t n = 0; n < N; n += 1) {
+		BOOST_REQUIRE_EQUAL(fuse->write("/test", (const char *)(&n), s, n, &fi), s);
+	}
+	BOOST_REQUIRE_EQUAL(fuse->fgetattr("/test", &st, &fi), 0);
+	BOOST_REQUIRE_EQUAL(st.st_size, N + s - 1);
+	char buf[s];
+	BOOST_REQUIRE_EQUAL(fuse->read("/test", buf, s, (N - 1), &fi), s);
+	BOOST_REQUIRE_EQUAL(*(int*)buf, N - 1);
+	BOOST_REQUIRE_EQUAL(fuse->release("/test", &fi), 0);
+	BOOST_REQUIRE_EQUAL(fuse->getattr("/test", &st), 0);
+	BOOST_REQUIRE_EQUAL(st.st_size, N + s - 1);
+}
+
+BOOST_AUTO_TEST_CASE( bgwriteDistinct, * boost::unit_test::timeout(2) )
+{
+	struct fuse_file_info fi;
+	memset(&fi, 0, sizeof(fi));
+	struct stat st;
+	memset(&st, 0, sizeof(st));
+	fi.flags = O_RDWR;
+	auto s = sizeof(int32_t);
+	auto N = 20;
+	BOOST_REQUIRE_EQUAL(fuse->create("/test", 0600, &fi), 0);
+	for (int32_t n = 0; n < N; n += 1) {
+		BOOST_REQUIRE_EQUAL(fuse->write("/test", (const char *)(&n), s, n * s, &fi), s);
+	}
+	BOOST_REQUIRE_EQUAL(fuse->fgetattr("/test", &st, &fi), 0);
+	BOOST_REQUIRE_EQUAL(st.st_size, N * s);
+	char buf[s];
+	BOOST_REQUIRE_EQUAL(fuse->read("/test", buf, s, (N - 1) * s, &fi), s);
+	BOOST_REQUIRE_EQUAL(*(int*)buf, N - 1);
+	BOOST_REQUIRE_EQUAL(fuse->release("/test", &fi), 0);
+	BOOST_REQUIRE_EQUAL(fuse->getattr("/test", &st), 0);
+	BOOST_REQUIRE_EQUAL(st.st_size, N * s);
+}
+
 BOOST_AUTO_TEST_CASE( symlinks )
 {
 	char buf[BUFSIZ];
-- 
cgit v1.2.3