diff options
Diffstat (limited to '')
| -rw-r--r-- | NEWS | 2 | ||||
| -rw-r--r-- | src/lib/nfs/Manager.cxx | 30 | ||||
| -rw-r--r-- | src/lib/nfs/Manager.hxx | 33 |
3 files changed, 56 insertions, 9 deletions
@@ -1,4 +1,6 @@ ver 0.19.5 (not yet released) +* input + - nfs: fix crash on connection failure * decoder - dsdiff, dsf, opus: fix deadlock while seeking - mp4v2: remove because of incompatible license 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 |