From 6e3f71f2eedd0bd18b3930e1a0eeb7d06a4399fa Mon Sep 17 00:00:00 2001
From: Dan Goodliffe <dan@randomdan.homeip.net>
Date: Sat, 5 Jan 2019 16:39:00 +0000
Subject: Take a shared lock of global state during regular operation

Fixes issue where proxies might be replaced whilst in-use.
Splits file/dir proxy map lock.
---
 netfs/fuse/fuseApp.cpp            |  2 +-
 netfs/fuse/fuseApp.h              |  2 +-
 netfs/fuse/fuseApp.impl.h         |  6 +++---
 netfs/fuse/fuseAppBase.h          |  6 ++++++
 netfs/unittests/testEdgeCases.cpp | 37 +++++++++++++++++++++++++++++++++++++
 5 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/netfs/fuse/fuseApp.cpp b/netfs/fuse/fuseApp.cpp
index a5dd0b1..d4835b9 100644
--- a/netfs/fuse/fuseApp.cpp
+++ b/netfs/fuse/fuseApp.cpp
@@ -150,7 +150,6 @@ NetFS::FuseApp::connectToService()
 {
 	if (!service) {
 		Lock(_lock);
-
 		auto proxyAddr = fcr->ServiceIdentity;
 		for (const auto & ep : fcr->Endpoints) {
 			proxyAddr += ":" + ep;
@@ -166,6 +165,7 @@ void
 NetFS::FuseApp::connectToVolume()
 {
 	if (!volume) {
+		Lock(_lock);
 		volume = service->connect(fcr->ExportName, fcr->AuthToken);
 		if (!volume) {
 			throw std::runtime_error("Invalid filesystem proxy");
diff --git a/netfs/fuse/fuseApp.h b/netfs/fuse/fuseApp.h
index 35c56dd..9653a55 100644
--- a/netfs/fuse/fuseApp.h
+++ b/netfs/fuse/fuseApp.h
@@ -124,7 +124,7 @@ namespace NetFS {
 			Ice::StringSeq args;
 			Ice::CommunicatorPtr ic;
 			Client::ResourcePtr fcr;
-			mutable std::shared_mutex _lock;
+			mutable std::shared_mutex _proxymaplock;
 
 			NetFS::VolumePrxPtr volume;
 			NetFS::ServicePrxPtr service;
diff --git a/netfs/fuse/fuseApp.impl.h b/netfs/fuse/fuseApp.impl.h
index 9d73de0..8dd9f1e 100644
--- a/netfs/fuse/fuseApp.impl.h
+++ b/netfs/fuse/fuseApp.impl.h
@@ -11,7 +11,7 @@ namespace NetFS {
 	FuseApp::setProxy(uint64_t & fh, const Params & ... params)
 	{
 		auto & map = getMap<Handle>();
-		Lock(_lock);
+		Lock(_proxymaplock);
 		while (map.find(fh = ++openHandleId) != map.end()) ;
 		map.emplace(fh, std::make_shared<typename Handle::element_type>(params...));
 	}
@@ -21,7 +21,7 @@ namespace NetFS {
 	FuseApp::getProxy(uint64_t localID)
 	{
 		const auto & map = getMap<Handle>();
-		SharedLock(_lock);
+		SharedLock(_proxymaplock);
 		auto i = map.find(localID);
 		if (i != map.end()) {
 			return i->second;
@@ -34,7 +34,7 @@ namespace NetFS {
 	FuseApp::clearProxy(uint64_t localID)
 	{
 		auto & map = getMap<Handle>();
-		Lock(_lock);
+		Lock(_proxymaplock);
 		map.erase(localID);
 	}
 }
diff --git a/netfs/fuse/fuseAppBase.h b/netfs/fuse/fuseAppBase.h
index 5cc8f7f..f0866bb 100644
--- a/netfs/fuse/fuseAppBase.h
+++ b/netfs/fuse/fuseAppBase.h
@@ -10,6 +10,8 @@
 #include <syslog.h>
 #include <visibility.h>
 #include <buffer.h>
+#include <shared_mutex>
+#include <lockHelpers.h>
 
 class DLL_PUBLIC FuseAppBase {
 	public:
@@ -145,6 +147,7 @@ class DLL_PUBLIC FuseAppBase {
 					for (int t = 0; ; ++t) {
 						try {
 							fuseApp->beforeOperation();
+							SharedLock(fuseApp->_lock);
 							return (fuseApp->*bfunc)(a...);
 						}
 						catch (const std::exception & ex) {
@@ -174,6 +177,9 @@ class DLL_PUBLIC FuseAppBase {
 		}
 
 		static FuseAppBase * fuseApp;
+
+	protected:
+		mutable std::shared_mutex _lock;
 };
 
 #endif
diff --git a/netfs/unittests/testEdgeCases.cpp b/netfs/unittests/testEdgeCases.cpp
index aeaa725..704d131 100644
--- a/netfs/unittests/testEdgeCases.cpp
+++ b/netfs/unittests/testEdgeCases.cpp
@@ -94,3 +94,40 @@ BOOST_AUTO_TEST_CASE ( daemonUnavailableAfterUse )
 	BOOST_REQUIRE_EQUAL(-EIO, fuse.fuse->statfs("/",  &s));
 }
 
+BOOST_AUTO_TEST_CASE( manyThreads )
+{
+	MockDaemonHost daemon(testEndpoint, {
+			"--NetFSD.ConfigPath=" + (rootDir / "defaultDaemon.xml").string()
+		});
+	FuseMockHost fuse(testEndpoint, {
+			(rootDir / "defaultFuse.xml:testvol").string(),
+			(rootDir / "test").string()
+		});
+
+	bool running = true;
+	std::vector<std::thread> ths;
+	std::atomic_uint success = 0;
+	for (int x = 0; x < 20; x++) {
+		ths.emplace_back([&] {
+			struct statvfs s;
+			while(running) {
+				if (fuse.fuse->statfs("/", &s) == 0) {
+					success++;
+				}
+			}
+			usleep(10000);
+		});
+	}
+
+	sleep(1);
+	daemon.restart();
+	sleep(1);
+	running = false;
+	
+	for (auto & th : ths) {
+		th.join();
+	}
+
+	BOOST_CHECK_GT(success, 800);
+}
+
-- 
cgit v1.2.3