Merge tag 'v0.19.5'

5 files changed, 106 insertions, 14 deletions

diff --git a/src/lib/nfs/Connection.cxx b/src/lib/nfs/Connection.cxx
index c2c7ceb2b..06d2a4d2a 100644
--- a/src/lib/nfs/Connection.cxx
+++ b/src/lib/nfs/Connection.cxx
@@ -327,6 +327,10 @@ NfsConnection::DestroyContext()
 	assert(GetEventLoop().IsInside());
 	assert(context != nullptr);
 
+	/* cancel pending DeferredMonitor that was scheduled to notify
+	   new leases */
+	DeferredMonitor::Cancel();
+
 	if (SocketMonitor::IsDefined())
 		SocketMonitor::Cancel();
 
@@ -405,10 +409,10 @@ NfsConnection::OnSocketReady(unsigned flags)
 		error.Format(nfs_domain, "NFS connection has failed: %s",
 			     nfs_get_error(context));
 
+		BroadcastError(std::move(error));
+
 		DestroyContext();
 		closed = true;
-
-		BroadcastError(std::move(error));
 	} else if (SocketMonitor::IsDefined() && nfs_get_fd(context) < 0) {
 		/* this happens when rpc_reconnect_requeue() is called
 		   after the connection broke, but autoreconnet was
@@ -421,10 +425,10 @@ NfsConnection::OnSocketReady(unsigned flags)
 			error.Format(nfs_domain,
 				     "NFS socket disappeared: %s", msg);
 
+		BroadcastError(std::move(error));
+
 		DestroyContext();
 		closed = true;
-
-		BroadcastError(std::move(error));
 	}
 
 	assert(in_event);
diff --git a/src/lib/nfs/FileReader.cxx b/src/lib/nfs/FileReader.cxx
index 4837e1f0e..1b80f2c86 100644
--- a/src/lib/nfs/FileReader.cxx
+++ b/src/lib/nfs/FileReader.cxx
@@ -56,8 +56,18 @@ NfsFileReader::Close()
 		return;
 	}
 
+	/* this cancels State::MOUNT */
 	connection->RemoveLease(*this);
 
+	CancelOrClose();
+}
+
+void
+NfsFileReader::CancelOrClose()
+{
+	assert(state != State::INITIAL &&
+	       state != State::DEFER);
+
 	if (state == State::IDLE)
 		/* no async operation in progress: can close
 		   immediately */
@@ -164,6 +174,8 @@ NfsFileReader::OnNfsConnectionFailed(const Error &error)
 {
 	assert(state == State::MOUNT);
 
+	state = State::INITIAL;
+
 	Error copy;
 	copy.Set(error);
 	OnNfsFileError(std::move(copy));
@@ -174,7 +186,7 @@ NfsFileReader::OnNfsConnectionDisconnected(const Error &error)
 {
 	assert(state > State::MOUNT);
 
-	state = State::INITIAL;
+	CancelOrClose();
 
 	Error copy;
 	copy.Set(error);
@@ -246,6 +258,30 @@ NfsFileReader::OnNfsCallback(unsigned status, void *data)
 void
 NfsFileReader::OnNfsError(Error &&error)
 {
+	switch (state) {
+	case State::INITIAL:
+	case State::DEFER:
+	case State::MOUNT:
+	case State::IDLE:
+		assert(false);
+		gcc_unreachable();
+
+	case State::OPEN:
+		connection->RemoveLease(*this);
+		state = State::INITIAL;
+		break;
+
+	case State::STAT:
+		connection->RemoveLease(*this);
+		connection->Close(fh);
+		state = State::INITIAL;
+		break;
+
+	case State::READ:
+		state = State::IDLE;
+		break;
+	}
+
 	OnNfsFileError(std::move(error));
 }
 
diff --git a/src/lib/nfs/FileReader.hxx b/src/lib/nfs/FileReader.hxx
index 7f43e0ecf..1495a2832 100644
--- a/src/lib/nfs/FileReader.hxx
+++ b/src/lib/nfs/FileReader.hxx
@@ -24,6 +24,7 @@
 #include "Lease.hxx"
 #include "Callback.hxx"
 #include "event/DeferredMonitor.hxx"
+#include "Compiler.h"
 
 #include <string>
 
@@ -75,6 +76,12 @@ protected:
 	virtual void OnNfsFileError(Error &&error) = 0;
 
 private:
+	/**
+	 * Cancel the current operation, if any.  The NfsLease must be
+	 * unregistered already.
+	 */
+	void CancelOrClose();
+
 	void OpenCallback(nfsfh *_fh);
 	void StatCallback(const struct stat *st);
 
diff --git a/src/lib/nfs/Manager.cxx b/src/lib/nfs/Manager.cxx
index c5aecf48d..6d50cce18 100644
--- a/src/lib/nfs/Manager.cxx
+++ b/src/lib/nfs/Manager.cxx
@@ -29,8 +29,10 @@ NfsManager::ManagedConnection::OnNfsConnectionError(Error &&error)
 {
 	FormatError(error, "NFS error on %s:%s", GetServer(), GetExportName());
 
-	manager.connections.erase(manager.connections.iterator_to(*this));
-	delete this;
+	/* defer deletion so the caller
+	   (i.e. NfsConnection::OnSocketReady()) can still use this
+	   object */
+	manager.ScheduleDelete(*this);
 }
 
 inline bool
@@ -59,7 +61,9 @@ NfsManager::Compare::operator()(const ManagedConnection &a,
 
 NfsManager::~NfsManager()
 {
-	assert(loop.IsInside());
+	assert(GetEventLoop().IsInside());
+
+	CollectGarbage();
 
 	connections.clear_and_dispose([](ManagedConnection *c){
 			delete c;
@@ -71,13 +75,13 @@ NfsManager::GetConnection(const char *server, const char *export_name)
 {
 	assert(server != nullptr);
 	assert(export_name != nullptr);
-	assert(loop.IsInside());
+	assert(GetEventLoop().IsInside());
 
 	Map::insert_commit_data hint;
 	auto result = connections.insert_check(LookupKey{server, export_name},
 					       Compare(), hint);
 	if (result.second) {
-		auto c = new ManagedConnection(*this, loop,
+		auto c = new ManagedConnection(*this, GetEventLoop(),
 					       server, export_name);
 		connections.insert_commit(*c, hint);
 		return *c;
@@ -85,3 +89,19 @@ NfsManager::GetConnection(const char *server, const char *export_name)
 		return *result.first;
 	}
 }
+
+void
+NfsManager::CollectGarbage()
+{
+	assert(GetEventLoop().IsInside());
+
+	garbage.clear_and_dispose([](ManagedConnection *c){
+			delete c;
+		});
+}
+
+void
+NfsManager::OnIdle()
+{
+	CollectGarbage();
+}
diff --git a/src/lib/nfs/Manager.hxx b/src/lib/nfs/Manager.hxx
index 612b01f9c..130c81aca 100644
--- a/src/lib/nfs/Manager.hxx
+++ b/src/lib/nfs/Manager.hxx
@@ -23,14 +23,16 @@
 #include "check.h"
 #include "Connection.hxx"
 #include "Compiler.h"
+#include "event/IdleMonitor.hxx"
 
 #include <boost/intrusive/set.hpp>
+#include <boost/intrusive/slist.hpp>
 
 /**
  * A manager for NFS connections.  Handles multiple connections to
  * multiple NFS servers.
  */
-class NfsManager {
+class NfsManager final : IdleMonitor {
 	struct LookupKey {
 		const char *server;
 		const char *export_name;
@@ -38,6 +40,7 @@ class NfsManager {
 
 	class ManagedConnection final
 		: public NfsConnection,
+		  public boost::intrusive::slist_base_hook<boost::intrusive::link_mode<boost::intrusive::normal_link>>,
 		  public boost::intrusive::set_base_hook<boost::intrusive::link_mode<boost::intrusive::normal_link>> {
 		NfsManager &manager;
 
@@ -63,8 +66,6 @@ class NfsManager {
 				const LookupKey b) const;
 	};
 
-	EventLoop &loop;
-
 	/**
 	 * Maps server and export_name to #ManagedConnection.
 	 */
@@ -74,9 +75,18 @@ class NfsManager {
 
 	Map connections;
 
+	typedef boost::intrusive::slist<ManagedConnection> List;
+
+	/**
+	 * A list of "garbage" connection objects.  Their destruction
+	 * is postponed because they were thrown into the garbage list
+	 * when callers on the stack were still using them.
+	 */
+	List garbage;
+
 public:
 	NfsManager(EventLoop &_loop)
-		:loop(_loop) {}
+		:IdleMonitor(_loop) {}
 
 	/**
 	 * Must be run from EventLoop's thread.
@@ -86,6 +96,21 @@ public:
 	gcc_pure
 	NfsConnection &GetConnection(const char *server,
 				     const char *export_name);
+
+private:
+	void ScheduleDelete(ManagedConnection &c) {
+		connections.erase(connections.iterator_to(c));
+		garbage.push_front(c);
+		IdleMonitor::Schedule();
+	}
+
+	/**
+	 * Delete all connections on the #garbage list.
+	 */
+	void CollectGarbage();
+
+	/* virtual methods from IdleMonitor */
+	void OnIdle() override;
 };
 
 #endif