event/IdleMonitor: refactor to IdleEvent

Instead of using this as a base class implementing a virtual method,
the new class IdleEvent can be used as a variable, decoupling
IdleMonitor's internal state from the derived class.

This is similar to commit 30a5dd267b
which refactored TimeoutMonitor to TimerEvent.

src/db/plugins/ProxyDatabasePlugin.cxx

@@ -43,7 +43,7 @@
 #include "util/RuntimeError.hxx"
 #include "protocol/Ack.hxx"
 #include "event/SocketMonitor.hxx"
-#include "event/IdleMonitor.hxx"
+#include "event/IdleEvent.hxx"
 #include "Log.hxx"
 
 #include <mpd/client.h>
@@ -85,7 +85,9 @@ public:
 	}
 };
 
-class ProxyDatabase final : public Database, SocketMonitor, IdleMonitor {
+class ProxyDatabase final : public Database, SocketMonitor {
+	IdleEvent idle_event;
+
 	DatabaseListener &listener;
 
 	const std::string host;
@@ -147,11 +149,10 @@ private:
 
 	void Disconnect() noexcept;
 
+	void OnIdle() noexcept;
+
 	/* virtual methods from SocketMonitor */
 	bool OnSocketReady(unsigned flags) noexcept override;
-
-	/* virtual methods from IdleMonitor */
-	void OnIdle() noexcept override;
 };
 
 static constexpr struct {
@@ -445,7 +446,8 @@ SendGroup(mpd_connection *connection, ConstBuffer<TagType> group)
 ProxyDatabase::ProxyDatabase(EventLoop &_loop, DatabaseListener &_listener,
 			     const ConfigBlock &block)
 	:Database(proxy_db_plugin),
-	 SocketMonitor(_loop), IdleMonitor(_loop),
+	 SocketMonitor(_loop),
+	 idle_event(_loop, BIND_THIS_METHOD(OnIdle)),
 	 listener(_listener),
 	 host(block.GetBlockValue("host", "")),
 	 password(block.GetBlockValue("password", "")),
@@ -526,7 +528,7 @@ ProxyDatabase::Connect()
 	is_idle = false;
 
 	SocketMonitor::Open(SocketDescriptor(mpd_async_get_fd(mpd_connection_get_async(connection))));
-	IdleMonitor::Schedule();
+	idle_event.Schedule();
 }
 
 void
@@ -553,7 +555,7 @@ ProxyDatabase::CheckConnection()
 
 		idle_received |= idle;
 		is_idle = false;
-		IdleMonitor::Schedule();
+		idle_event.Schedule();
 	}
 }
 
@@ -571,7 +573,7 @@ ProxyDatabase::Disconnect() noexcept
 {
 	assert(connection != nullptr);
 
-	IdleMonitor::Cancel();
+	idle_event.Cancel();
 	SocketMonitor::Steal();
 
 	mpd_connection_free(connection);
@@ -585,7 +587,7 @@ ProxyDatabase::OnSocketReady([[maybe_unused]] unsigned flags) noexcept
 
 	if (!is_idle) {
 		// TODO: can this happen?
-		IdleMonitor::Schedule();
+		idle_event.Schedule();
 		SocketMonitor::Cancel();
 		return true;
 	}
@@ -604,7 +606,7 @@ ProxyDatabase::OnSocketReady([[maybe_unused]] unsigned flags) noexcept
 	/* let OnIdle() handle this */
 	idle_received |= idle;
 	is_idle = false;
-	IdleMonitor::Schedule();
+	idle_event.Schedule();
 	SocketMonitor::Cancel();
 	return true;
 }

src/event/FullyBufferedSocket.cxx

@@ -34,7 +34,7 @@ FullyBufferedSocket::DirectWrite(const void *data, size_t length) noexcept
 		if (IsSocketErrorAgain(code))
 			return 0;
 
-		IdleMonitor::Cancel();
+		idle_event.Cancel();
 		BufferedSocket::Cancel();
 
 		if (IsSocketErrorClosed(code))
@@ -53,7 +53,7 @@ FullyBufferedSocket::Flush() noexcept
 
 	const auto data = output.Read();
 	if (data.empty()) {
-		IdleMonitor::Cancel();
+		idle_event.Cancel();
 		CancelWrite();
 		return true;
 	}
@@ -65,7 +65,7 @@ FullyBufferedSocket::Flush() noexcept
 	output.Consume(nbytes);
 
 	if (output.empty()) {
-		IdleMonitor::Cancel();
+		idle_event.Cancel();
 		CancelWrite();
 	}
 
@@ -88,7 +88,7 @@ FullyBufferedSocket::Write(const void *data, size_t length) noexcept
 	}
 
 	if (was_empty)
-		IdleMonitor::Schedule();
+		idle_event.Schedule();
 	return true;
 }
 
@@ -97,7 +97,7 @@ FullyBufferedSocket::OnSocketReady(unsigned flags) noexcept
 {
 	if (flags & WRITE) {
 		assert(!output.empty());
-		assert(!IdleMonitor::IsActive());
+		assert(!idle_event.IsActive());
 
 		if (!Flush())
 			return false;

src/event/FullyBufferedSocket.hxx

@@ -21,19 +21,22 @@
 #define MPD_FULLY_BUFFERED_SOCKET_HXX
 
 #include "BufferedSocket.hxx"
-#include "IdleMonitor.hxx"
+#include "IdleEvent.hxx"
 #include "util/PeakBuffer.hxx"
 
 /**
  * A #BufferedSocket specialization that adds an output buffer.
  */
-class FullyBufferedSocket : protected BufferedSocket, private IdleMonitor {
+class FullyBufferedSocket : protected BufferedSocket {
+	IdleEvent idle_event;
+
 	PeakBuffer output;
 
 public:
 	FullyBufferedSocket(SocketDescriptor _fd, EventLoop &_loop,
 			    size_t normal_size, size_t peak_size=0) noexcept
-		:BufferedSocket(_fd, _loop), IdleMonitor(_loop),
+		:BufferedSocket(_fd, _loop),
+		 idle_event(_loop, BIND_THIS_METHOD(OnIdle)),
 		 output(normal_size, peak_size) {
 	}
 
@@ -41,7 +44,7 @@ public:
 	using BufferedSocket::IsDefined;
 
 	void Close() noexcept {
-		IdleMonitor::Cancel();
+		idle_event.Cancel();
 		BufferedSocket::Close();
 	}
 
@@ -69,7 +72,7 @@ protected:
 	/* virtual methods from class SocketMonitor */
 	bool OnSocketReady(unsigned flags) noexcept override;
 
-	void OnIdle() noexcept override;
+	void OnIdle() noexcept;
 };
 
 #endif

src/event/IdleEvent.cxx

@@ -17,13 +17,13 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#include "IdleMonitor.hxx"
+#include "IdleEvent.hxx"
 #include "Loop.hxx"
 
 #include <cassert>
 
 void
-IdleMonitor::Cancel() noexcept
+IdleEvent::Cancel() noexcept
 {
 	assert(loop.IsInside());
 
@@ -34,7 +34,7 @@ IdleMonitor::Cancel() noexcept
 }
 
 void
-IdleMonitor::Schedule() noexcept
+IdleEvent::Schedule() noexcept
 {
 	assert(loop.IsInside());
 
@@ -46,9 +46,9 @@ IdleMonitor::Schedule() noexcept
 }
 
 void
-IdleMonitor::Run() noexcept
+IdleEvent::Run() noexcept
 {
 	assert(loop.IsInside());
 
-	OnIdle();
+	callback();
 }

src/event/IdleEvent.hxx

@@ -17,8 +17,10 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
-#ifndef MPD_SOCKET_IDLE_MONITOR_HXX
+#ifndef MPD_SOCKET_IDLE_EVENT_HXX
-#define MPD_SOCKET_IDLE_MONITOR_HXX
+#define MPD_SOCKET_IDLE_EVENT_HXX
+
+#include "util/BindMethod.hxx"
 
 #include <boost/intrusive/list_hook.hpp>
 
@@ -32,19 +34,22 @@ class EventLoop;
  * thread that runs the #EventLoop, except where explicitly documented
  * as thread-safe.
  */
-class IdleMonitor {
+class IdleEvent {
 	friend class EventLoop;
 
-	typedef boost::intrusive::list_member_hook<> ListHook;
+	using ListHook = boost::intrusive::list_member_hook<>;
 	ListHook list_hook;
 
 	EventLoop &loop;
 
-public:
+	using Callback = BoundMethod<void() noexcept>;
-	explicit IdleMonitor(EventLoop &_loop) noexcept
+	const Callback callback;
-		:loop(_loop) {}
 
-	~IdleMonitor() noexcept {
+public:
+	IdleEvent(EventLoop &_loop, Callback _callback) noexcept
+		:loop(_loop), callback(_callback) {}
+
+	~IdleEvent() noexcept {
 #ifndef NDEBUG
 		/* this check is redundant, it is only here to avoid
 		   the assertion in Cancel() */
@@ -64,9 +69,6 @@ public:
 	void Schedule() noexcept;
 	void Cancel() noexcept;
 
-protected:
-	virtual void OnIdle() noexcept = 0;
-
 private:
 	void Run() noexcept;
 };

src/event/Loop.cxx

@@ -20,7 +20,7 @@
 #include "Loop.hxx"
 #include "TimerEvent.hxx"
 #include "SocketMonitor.hxx"
-#include "IdleMonitor.hxx"
+#include "IdleEvent.hxx"
 #include "DeferEvent.hxx"
 #include "util/ScopeExit.hxx"
 
@@ -103,7 +103,7 @@ EventLoop::RemoveFD(int _fd) noexcept
 }
 
 void
-EventLoop::AddIdle(IdleMonitor &i) noexcept
+EventLoop::AddIdle(IdleEvent &i) noexcept
 {
 	assert(IsInside());
 
@@ -112,7 +112,7 @@ EventLoop::AddIdle(IdleMonitor &i) noexcept
 }
 
 void
-EventLoop::RemoveIdle(IdleMonitor &i) noexcept
+EventLoop::RemoveIdle(IdleEvent &i) noexcept
 {
 	assert(IsInside());
 
@@ -204,7 +204,7 @@ EventLoop::Run() noexcept
 		/* invoke idle */
 
 		while (!idle.empty()) {
-			IdleMonitor &m = idle.front();
+			IdleEvent &m = idle.front();
 			idle.pop_front();
 			m.Run();
 
@@ -221,7 +221,7 @@ EventLoop::Run() noexcept
 
 			if (again)
 				/* re-evaluate timers because one of
-				   the IdleMonitors may have added a
+				   the IdleEvents may have added a
 				   new timeout */
 				continue;
 		}

src/event/Loop.hxx

@@ -24,7 +24,7 @@
 #include "PollGroup.hxx"
 #include "WakeFD.hxx"
 #include "SocketMonitor.hxx"
-#include "IdleMonitor.hxx"
+#include "IdleEvent.hxx"
 #include "DeferEvent.hxx"
 #include "thread/Id.hxx"
 #include "thread/Mutex.hxx"
@@ -52,7 +52,7 @@ class TimerEvent;
  * thread that runs it, except where explicitly documented as
  * thread-safe.
  *
- * @see SocketMonitor, MultiSocketMonitor, TimerEvent, IdleMonitor
+ * @see SocketMonitor, MultiSocketMonitor, TimerEvent, IdleEvent
  */
 class EventLoop final : SocketMonitor
 {
@@ -71,10 +71,10 @@ class EventLoop final : SocketMonitor
 	TimerSet timers;
 
 	using IdleList =
-		boost::intrusive::list<IdleMonitor,
+		boost::intrusive::list<IdleEvent,
-				       boost::intrusive::member_hook<IdleMonitor,
+				       boost::intrusive::member_hook<IdleEvent,
-								     IdleMonitor::ListHook,
+								     IdleEvent::ListHook,
-								     &IdleMonitor::list_hook>,
+								     &IdleEvent::list_hook>,
 				       boost::intrusive::constant_time_size<false>>;
 	IdleList idle;
 
@@ -194,8 +194,8 @@ public:
 
 	bool RemoveFD(int fd) noexcept;
 
-	void AddIdle(IdleMonitor &i) noexcept;
+	void AddIdle(IdleEvent &i) noexcept;
-	void RemoveIdle(IdleMonitor &i) noexcept;
+	void RemoveIdle(IdleEvent &i) noexcept;
 
 	void AddTimer(TimerEvent &t, Event::Duration d) noexcept;
 

src/event/MultiSocketMonitor.cxx

@@ -31,7 +31,7 @@
 #endif
 
 MultiSocketMonitor::MultiSocketMonitor(EventLoop &_loop) noexcept
-	:IdleMonitor(_loop),
+	:idle_event(_loop, BIND_THIS_METHOD(OnIdle)),
 	 timeout_event(_loop, BIND_THIS_METHOD(OnTimeout)) {
 }
 
@@ -44,7 +44,7 @@ MultiSocketMonitor::Reset() noexcept
 #ifdef USE_EPOLL
 	always_ready_fds.clear();
 #endif
-	IdleMonitor::Cancel();
+	idle_event.Cancel();
 	timeout_event.Cancel();
 	ready = refresh = false;
 }

src/event/MultiSocketMonitor.hxx

@@ -20,7 +20,7 @@
 #ifndef MPD_MULTI_SOCKET_MONITOR_HXX
 #define MPD_MULTI_SOCKET_MONITOR_HXX
 
-#include "IdleMonitor.hxx"
+#include "IdleEvent.hxx"
 #include "TimerEvent.hxx"
 #include "SocketMonitor.hxx"
 #include "event/Features.h"
@@ -41,7 +41,7 @@ class EventLoop;
  * In PrepareSockets(), use UpdateSocketList() and AddSocket().
  * DispatchSockets() will be called if at least one socket is ready.
  */
-class MultiSocketMonitor : IdleMonitor
+class MultiSocketMonitor
 {
 	class SingleFD final : public SocketMonitor {
 		MultiSocketMonitor &multi;
@@ -83,6 +83,8 @@ class MultiSocketMonitor : IdleMonitor
 		}
 	};
 
+	IdleEvent idle_event;
+
 	TimerEvent timeout_event;
 
 	/**
@@ -124,7 +126,9 @@ public:
 
 	MultiSocketMonitor(EventLoop &_loop) noexcept;
 
-	using IdleMonitor::GetEventLoop;
+	EventLoop &GetEventLoop() const noexcept {
+		return idle_event.GetEventLoop();
+	}
 
 	/**
 	 * Clear the socket list and disable all #EventLoop
@@ -149,7 +153,7 @@ public:
 	 */
 	void InvalidateSockets() noexcept {
 		refresh = true;
-		IdleMonitor::Schedule();
+		idle_event.Schedule();
 	}
 
 	/**
@@ -238,7 +242,7 @@ protected:
 private:
 	void SetReady() noexcept {
 		ready = true;
-		IdleMonitor::Schedule();
+		idle_event.Schedule();
 	}
 
 	void Prepare() noexcept;
@@ -247,7 +251,7 @@ private:
 		SetReady();
 	}
 
-	virtual void OnIdle() noexcept final;
+	void OnIdle() noexcept;
 };
 
 #endif

src/event/UringManager.hxx

@@ -20,18 +20,20 @@
 #pragma once
 
 #include "SocketMonitor.hxx"
-#include "IdleMonitor.hxx"
+#include "IdleEvent.hxx"
 #include "io/uring/Queue.hxx"
 
 namespace Uring {
 
-class Manager final : public Queue, SocketMonitor, IdleMonitor {
+class Manager final : public Queue, SocketMonitor {
+	IdleEvent idle_event;
+
 public:
 	explicit Manager(EventLoop &event_loop)
 		:Queue(1024, 0),
 		 SocketMonitor(SocketDescriptor::FromFileDescriptor(GetFileDescriptor()),
 			       event_loop),
-		 IdleMonitor(event_loop)
+		 idle_event(event_loop, BIND_THIS_METHOD(OnIdle))
 	{
 		SocketMonitor::ScheduleRead();
 	}
@@ -39,12 +41,12 @@ public:
 	void Push(struct io_uring_sqe &sqe,
 		  Operation &operation) noexcept override {
 		AddPending(sqe, operation);
-		IdleMonitor::Schedule();
+		idle_event.Schedule();
 	}
 
 private:
 	bool OnSocketReady(unsigned flags) noexcept override;
-	void OnIdle() noexcept override;
+	void OnIdle() noexcept;
 };
 
 } // namespace Uring

src/event/meson.build

@@ -22,7 +22,7 @@ event = static_library(
   'event',
   'SignalMonitor.cxx',
   'TimerEvent.cxx',
-  'IdleMonitor.cxx',
+  'IdleEvent.cxx',
   'DeferEvent.cxx',
   'MaskMonitor.cxx',
   'SocketMonitor.cxx',

src/lib/nfs/Manager.hxx

@@ -22,7 +22,7 @@
 
 #include "Connection.hxx"
 #include "util/Compiler.h"
-#include "event/IdleMonitor.hxx"
+#include "event/IdleEvent.hxx"
 
 #include <boost/intrusive/set.hpp>
 #include <boost/intrusive/slist.hpp>
@@ -31,7 +31,7 @@
  * A manager for NFS connections.  Handles multiple connections to
  * multiple NFS servers.
  */
-class NfsManager final : IdleMonitor {
+class NfsManager final {
 	struct LookupKey {
 		const char *server;
 		const char *export_name;
@@ -87,16 +87,20 @@ class NfsManager final : IdleMonitor {
 	 */
 	List garbage;
 
+	IdleEvent idle_event;
+
 public:
 	explicit NfsManager(EventLoop &_loop) noexcept
-		:IdleMonitor(_loop) {}
+		:idle_event(_loop, BIND_THIS_METHOD(OnIdle)) {}
 
 	/**
 	 * Must be run from EventLoop's thread.
 	 */
 	~NfsManager() noexcept;
 
-	using IdleMonitor::GetEventLoop;
+	auto &GetEventLoop() const noexcept {
+		return idle_event.GetEventLoop();
+	}
 
 	gcc_pure
 	NfsConnection &GetConnection(const char *server,
@@ -106,7 +110,7 @@ private:
 	void ScheduleDelete(ManagedConnection &c) noexcept {
 		connections.erase(connections.iterator_to(c));
 		garbage.push_front(c);
-		IdleMonitor::Schedule();
+		idle_event.Schedule();
 	}
 
 	/**
@@ -115,7 +119,7 @@ private:
 	void CollectGarbage() noexcept;
 
 	/* virtual methods from IdleMonitor */
-	void OnIdle() noexcept override;
+	void OnIdle() noexcept;
 };
 
 #endif