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.
This commit is contained in:
		
							
								
								
									
										1
									
								
								NEWS
									
									
									
									
									
								
							
							
						
						
									
										1
									
								
								NEWS
									
									
									
									
									
								
							| @@ -1,6 +1,7 @@ | |||||||
| ver 0.19.7 (not yet released) | ver 0.19.7 (not yet released) | ||||||
| * input | * input | ||||||
|   - nfs: fix crash while canceling a failing file open operation |   - nfs: fix crash while canceling a failing file open operation | ||||||
|  |   - nfs: fix memory leak on connection failure | ||||||
| * playlist | * playlist | ||||||
|   - don't skip non-existent songs in "listplaylist" |   - don't skip non-existent songs in "listplaylist" | ||||||
| * fix memory allocator bug on Windows | * fix memory allocator bug on Windows | ||||||
|   | |||||||
| @@ -157,6 +157,12 @@ public: | |||||||
|  |  | ||||||
| 		return *i; | 		return *i; | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  | 	template<typename F> | ||||||
|  | 	void ForEach(F &&f) { | ||||||
|  | 		for (CT &i : list) | ||||||
|  | 			f(i); | ||||||
|  | 	} | ||||||
| }; | }; | ||||||
|  |  | ||||||
| #endif | #endif | ||||||
|   | |||||||
| @@ -132,6 +132,17 @@ NfsConnection::CancellableCallback::CancelAndScheduleClose(struct nfsfh *fh) | |||||||
| 	Cancel(); | 	Cancel(); | ||||||
| } | } | ||||||
|  |  | ||||||
|  | inline void | ||||||
|  | NfsConnection::CancellableCallback::PrepareDestroyContext() | ||||||
|  | { | ||||||
|  | 	assert(IsCancelled()); | ||||||
|  |  | ||||||
|  | 	if (close_fh != nullptr) { | ||||||
|  | 		connection.InternalClose(close_fh); | ||||||
|  | 		close_fh = nullptr; | ||||||
|  | 	} | ||||||
|  | } | ||||||
|  |  | ||||||
| inline void | inline void | ||||||
| NfsConnection::CancellableCallback::Callback(int err, void *data) | NfsConnection::CancellableCallback::Callback(int err, void *data) | ||||||
| { | { | ||||||
| @@ -370,6 +381,10 @@ NfsConnection::DestroyContext() | |||||||
| 	if (SocketMonitor::IsDefined()) | 	if (SocketMonitor::IsDefined()) | ||||||
| 		SocketMonitor::Cancel(); | 		SocketMonitor::Cancel(); | ||||||
|  |  | ||||||
|  | 	callbacks.ForEach([](CancellableCallback &c){ | ||||||
|  | 			c.PrepareDestroyContext(); | ||||||
|  | 		}); | ||||||
|  |  | ||||||
| 	nfs_destroy_context(context); | 	nfs_destroy_context(context); | ||||||
| 	context = nullptr; | 	context = nullptr; | ||||||
| } | } | ||||||
|   | |||||||
| @@ -84,6 +84,13 @@ class NfsConnection : SocketMonitor, DeferredMonitor { | |||||||
| 		 */ | 		 */ | ||||||
| 		void CancelAndScheduleClose(struct nfsfh *fh); | 		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: | 	private: | ||||||
| 		static void Callback(int err, struct nfs_context *nfs, | 		static void Callback(int err, struct nfs_context *nfs, | ||||||
| 				     void *data, void *private_data); | 				     void *data, void *private_data); | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Max Kellermann
					Max Kellermann