diff options
| author | Dan Goodliffe <dan@randomdan.homeip.net> | 2019-01-05 16:39:00 +0000 | 
|---|---|---|
| committer | Dan Goodliffe <dan@randomdan.homeip.net> | 2019-01-05 16:39:00 +0000 | 
| commit | 6e3f71f2eedd0bd18b3930e1a0eeb7d06a4399fa (patch) | |
| tree | 72744262a7c244ec6557bcdc0ba653fb7144f6ab | |
| parent | Tidy getHelper with constexpr and lambda (diff) | |
| download | netfs-6e3f71f2eedd0bd18b3930e1a0eeb7d06a4399fa.tar.bz2 netfs-6e3f71f2eedd0bd18b3930e1a0eeb7d06a4399fa.tar.xz netfs-6e3f71f2eedd0bd18b3930e1a0eeb7d06a4399fa.zip  | |
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.
| -rw-r--r-- | netfs/fuse/fuseApp.cpp | 2 | ||||
| -rw-r--r-- | netfs/fuse/fuseApp.h | 2 | ||||
| -rw-r--r-- | netfs/fuse/fuseApp.impl.h | 6 | ||||
| -rw-r--r-- | netfs/fuse/fuseAppBase.h | 6 | ||||
| -rw-r--r-- | 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); +} +  | 
