aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kellermann <max@duempel.org>2014-11-25 10:42:52 +0100
committerMax Kellermann <max@duempel.org>2014-11-25 13:31:18 +0100
commit3cef348f30ad8a018b1c46f257d3e719be3f96b0 (patch)
tree707c23f63f63838a5f7b4f895ae260a4e8bda48c
parentb293b160079fd19cfc675fa86c2b99695aac171f (diff)
downloadmpd-3cef348f30ad8a018b1c46f257d3e719be3f96b0.tar.gz
mpd-3cef348f30ad8a018b1c46f257d3e719be3f96b0.tar.xz
mpd-3cef348f30ad8a018b1c46f257d3e719be3f96b0.zip
lib/nfs/Manager: defer NfsConnection destruction
Avoids a crash that occurs when NfsConnection::OnSocketReady() dereferences itself before returning.
-rw-r--r--NEWS2
-rw-r--r--src/lib/nfs/Manager.cxx30
-rw-r--r--src/lib/nfs/Manager.hxx33
3 files changed, 56 insertions, 9 deletions
diff --git a/NEWS b/NEWS
index 7d04d528c..5d089033a 100644
--- a/NEWS
+++ b/NEWS
@@ -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