From a543627abd19f321ddd4259e222e6437e8312417 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Sun, 14 Dec 2014 15:56:53 +0100
Subject: [PATCH] lib/nfs/Connection: fix memory leak (and assertion failure)

nfs_destroy_context() will invoke all pending callbacks with
err==-EINTR.  In CancellableCallback::Callback(), this will invoke
NfsConnection::DeferClose(), which however is only designed to be
called from nfs_service().  In non-debug mode, this will leak memory
because nfs_close_async() is never called.

Workaround: before nfs_destroy_context(), invoke nfs_close_async() on
all pending file handles.
---
 NEWS                        |  1 +
 src/lib/nfs/Cancellable.hxx |  6 ++++++
 src/lib/nfs/Connection.cxx  | 15 +++++++++++++++
 src/lib/nfs/Connection.hxx  |  7 +++++++
 4 files changed, 29 insertions(+)

diff --git a/NEWS b/NEWS
index c1f967b74..6e83245c4 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,7 @@
 ver 0.19.7 (not yet released)
 * input
   - nfs: fix crash while canceling a failing file open operation
+  - nfs: fix memory leak on connection failure
 * playlist
   - don't skip non-existent songs in "listplaylist"
 * fix memory allocator bug on Windows
diff --git a/src/lib/nfs/Cancellable.hxx b/src/lib/nfs/Cancellable.hxx
index be4527ac3..151be0528 100644
--- a/src/lib/nfs/Cancellable.hxx
+++ b/src/lib/nfs/Cancellable.hxx
@@ -157,6 +157,12 @@ public:
 
 		return *i;
 	}
+
+	template<typename F>
+	void ForEach(F &&f) {
+		for (CT &i : list)
+			f(i);
+	}
 };
 
 #endif
diff --git a/src/lib/nfs/Connection.cxx b/src/lib/nfs/Connection.cxx
index 48db620be..83b388dc4 100644
--- a/src/lib/nfs/Connection.cxx
+++ b/src/lib/nfs/Connection.cxx
@@ -132,6 +132,17 @@ NfsConnection::CancellableCallback::CancelAndScheduleClose(struct nfsfh *fh)
 	Cancel();
 }
 
+inline void
+NfsConnection::CancellableCallback::PrepareDestroyContext()
+{
+	assert(IsCancelled());
+
+	if (close_fh != nullptr) {
+		connection.InternalClose(close_fh);
+		close_fh = nullptr;
+	}
+}
+
 inline void
 NfsConnection::CancellableCallback::Callback(int err, void *data)
 {
@@ -370,6 +381,10 @@ NfsConnection::DestroyContext()
 	if (SocketMonitor::IsDefined())
 		SocketMonitor::Cancel();
 
+	callbacks.ForEach([](CancellableCallback &c){
+			c.PrepareDestroyContext();
+		});
+
 	nfs_destroy_context(context);
 	context = nullptr;
 }
diff --git a/src/lib/nfs/Connection.hxx b/src/lib/nfs/Connection.hxx
index 3d872eb3a..e47ba404b 100644
--- a/src/lib/nfs/Connection.hxx
+++ b/src/lib/nfs/Connection.hxx
@@ -84,6 +84,13 @@ class NfsConnection : SocketMonitor, DeferredMonitor {
 		 */
 		void CancelAndScheduleClose(struct nfsfh *fh);
 
+		/**
+		 * Called by NfsConnection::DestroyContext() right
+		 * before nfs_destroy_context().  This object is given
+		 * a chance to prepare for the latter.
+		 */
+		void PrepareDestroyContext();
+
 	private:
 		static void Callback(int err, struct nfs_context *nfs,
 				     void *data, void *private_data);