aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMax Kellermann <max@duempel.org>2014-09-26 13:29:44 +0200
committerMax Kellermann <max@duempel.org>2014-10-01 23:10:32 +0200
commit0661fd6f7c66ae888b6fc253af6dd0514798eff5 (patch)
tree9cc213a9d3be7709f937df4cfb40255d74146338
parentedd003b62af802fae7828336628adb0ea3f6bd21 (diff)
downloadmpd-0661fd6f7c66ae888b6fc253af6dd0514798eff5.tar.gz
mpd-0661fd6f7c66ae888b6fc253af6dd0514798eff5.tar.xz
mpd-0661fd6f7c66ae888b6fc253af6dd0514798eff5.zip
lib/nfs/FileReader: postpone the nfs_close_async() call
If an async opertion is in progress, nfs_close_async() will make libnfs crash because the RPC callback will dereference an object that was freed by nfs_close_async().
Diffstat (limited to '')
-rw-r--r--src/lib/nfs/Cancellable.hxx7
-rw-r--r--src/lib/nfs/Connection.cxx43
-rw-r--r--src/lib/nfs/Connection.hxx33
-rw-r--r--src/lib/nfs/FileReader.cxx15
4 files changed, 92 insertions, 6 deletions
diff --git a/src/lib/nfs/Cancellable.hxx b/src/lib/nfs/Cancellable.hxx
index 1f287c329..be4527ac3 100644
--- a/src/lib/nfs/Cancellable.hxx
+++ b/src/lib/nfs/Cancellable.hxx
@@ -150,6 +150,13 @@ public:
i->Cancel();
}
+
+ CT &Get(reference_type p) {
+ auto i = Find(p);
+ assert(i != list.end());
+
+ return *i;
+ }
};
#endif
diff --git a/src/lib/nfs/Connection.cxx b/src/lib/nfs/Connection.cxx
index 853cacc9a..e5dc87727 100644
--- a/src/lib/nfs/Connection.cxx
+++ b/src/lib/nfs/Connection.cxx
@@ -81,9 +81,22 @@ NfsConnection::CancellableCallback::Read(nfs_context *ctx, struct nfsfh *fh,
}
inline void
+NfsConnection::CancellableCallback::CancelAndScheduleClose(struct nfsfh *fh)
+{
+ assert(!open);
+ assert(close_fh == nullptr);
+ assert(fh != nullptr);
+
+ close_fh = fh;
+ Cancel();
+}
+
+inline void
NfsConnection::CancellableCallback::Callback(int err, void *data)
{
if (!IsCancelled()) {
+ assert(close_fh == nullptr);
+
NfsCallback &cb = Get();
connection.callbacks.Remove(*this);
@@ -98,9 +111,12 @@ NfsConnection::CancellableCallback::Callback(int err, void *data)
/* a nfs_open_async() call was cancelled - to
avoid a memory leak, close the newly
allocated file handle immediately */
+ assert(close_fh == nullptr);
+
struct nfsfh *fh = (struct nfsfh *)data;
connection.Close(fh);
- }
+ } else if (close_fh != nullptr)
+ connection.DeferClose(close_fh);
connection.callbacks.Remove(*this);
}
@@ -135,6 +151,7 @@ NfsConnection::~NfsConnection()
assert(new_leases.empty());
assert(active_leases.empty());
assert(callbacks.IsEmpty());
+ assert(deferred_close.empty());
if (context != nullptr)
DestroyContext();
@@ -225,6 +242,13 @@ NfsConnection::Close(struct nfsfh *fh)
}
void
+NfsConnection::CancelAndClose(struct nfsfh *fh, NfsCallback &callback)
+{
+ CancellableCallback &cancel = callbacks.Get(callback);
+ cancel.CancelAndScheduleClose(fh);
+}
+
+void
NfsConnection::DestroyContext()
{
assert(GetEventLoop().IsInside());
@@ -237,6 +261,15 @@ NfsConnection::DestroyContext()
context = nullptr;
}
+inline void
+NfsConnection::DeferClose(struct nfsfh *fh)
+{
+ assert(in_event);
+ assert(in_service);
+
+ deferred_close.push_front(fh);
+}
+
void
NfsConnection::ScheduleSocket()
{
@@ -257,6 +290,8 @@ NfsConnection::ScheduleSocket()
bool
NfsConnection::OnSocketReady(unsigned flags)
{
+ assert(deferred_close.empty());
+
bool closed = false;
const bool was_mounted = mount_finished;
@@ -278,6 +313,12 @@ NfsConnection::OnSocketReady(unsigned flags)
assert(in_service);
in_service = false;
+ while (!deferred_close.empty()) {
+ nfs_close_async(context, deferred_close.front(),
+ DummyCallback, nullptr);
+ deferred_close.pop_front();
+ }
+
if (!was_mounted && mount_finished) {
if (postponed_mount_error.IsDefined()) {
DestroyContext();
diff --git a/src/lib/nfs/Connection.hxx b/src/lib/nfs/Connection.hxx
index 880b7d467..b3db37c5d 100644
--- a/src/lib/nfs/Connection.hxx
+++ b/src/lib/nfs/Connection.hxx
@@ -26,8 +26,11 @@
#include "event/DeferredMonitor.hxx"
#include "util/Error.hxx"
+#include <boost/intrusive/list.hpp>
+
#include <string>
#include <list>
+#include <forward_list>
struct nfs_context;
class NfsCallback;
@@ -47,13 +50,19 @@ class NfsConnection : SocketMonitor, DeferredMonitor {
*/
const bool open;
+ /**
+ * The file handle scheduled to be closed as soon as
+ * the operation finishes.
+ */
+ struct nfsfh *close_fh;
+
public:
explicit CancellableCallback(NfsCallback &_callback,
NfsConnection &_connection,
bool _open)
:CancellablePointer<NfsCallback>(_callback),
connection(_connection),
- open(_open) {}
+ open(_open), close_fh(nullptr) {}
bool Open(nfs_context *context, const char *path, int flags,
Error &error);
@@ -63,6 +72,12 @@ class NfsConnection : SocketMonitor, DeferredMonitor {
uint64_t offset, size_t size,
Error &error);
+ /**
+ * Cancel the operation and schedule a call to
+ * nfs_close_async() with the given file handle.
+ */
+ void CancelAndScheduleClose(struct nfsfh *fh);
+
private:
static void Callback(int err, struct nfs_context *nfs,
void *data, void *private_data);
@@ -79,6 +94,15 @@ class NfsConnection : SocketMonitor, DeferredMonitor {
typedef CancellableList<NfsCallback, CancellableCallback> CallbackList;
CallbackList callbacks;
+ /**
+ * A list of NFS file handles (struct nfsfh *) which shall be
+ * closed as soon as nfs_service() returns. If we close the
+ * file handle while in nfs_service(), libnfs may crash, and
+ * deferring this call to after nfs_service() avoids this
+ * problem.
+ */
+ std::forward_list<struct nfsfh *> deferred_close;
+
Error postponed_mount_error;
/**
@@ -135,6 +159,7 @@ public:
void Cancel(NfsCallback &callback);
void Close(struct nfsfh *fh);
+ void CancelAndClose(struct nfsfh *fh, NfsCallback &callback);
protected:
virtual void OnNfsConnectionError(Error &&error) = 0;
@@ -145,6 +170,12 @@ private:
}
void DestroyContext();
+
+ /**
+ * Invoke nfs_close_async() after nfs_service() returns.
+ */
+ void DeferClose(struct nfsfh *fh);
+
bool MountInternal(Error &error);
void BroadcastMountSuccess();
void BroadcastMountError(Error &&error);
diff --git a/src/lib/nfs/FileReader.cxx b/src/lib/nfs/FileReader.cxx
index 52d951fa6..d2be46f8e 100644
--- a/src/lib/nfs/FileReader.cxx
+++ b/src/lib/nfs/FileReader.cxx
@@ -57,11 +57,18 @@ NfsFileReader::Close()
connection->RemoveLease(*this);
- if (state > State::MOUNT && state != State::IDLE)
- connection->Cancel(*this);
-
- if (state > State::OPEN)
+ if (state == State::IDLE)
+ /* no async operation in progress: can close
+ immediately */
connection->Close(fh);
+ else if (state > State::OPEN)
+ /* one async operation in progress: cancel it and
+ defer the nfs_close_async() call */
+ connection->CancelAndClose(fh, *this);
+ else if (state > State::MOUNT)
+ /* we don't have a file handle yet - just cancel the
+ async operation */
+ connection->Cancel(*this);
state = State::INITIAL;
}