event/SocketMonitor: handle epoll_ctl()=EBADF/ENOENT in Schedule()

This fixes a freeze bug in the NFS input/storage plugins: when libnfs auto-reconnets after a failure, it installs the new socket on the same file descriptor number. MPD's attempt to unregister the old socket by calling SocketMonitor::Steal() from NfsConnection::ScheduleSocket() fails because the new/old socket number is not registered in epoll, so epoll_ctl() returns ENOENT. The problem is that it left `scheduled_flags`, and so subsequent Schedule() calls will use `EPOLL_CTL_MOD`, which will fail again and again. Instead, we need to use `EPOLL_CTL_ADD` to register the new socket. Closes https://github.com/MusicPlayerDaemon/MPD/issues/806 Closes https://github.com/MusicPlayerDaemon/MPD/issues/756

event/SocketMonitor: handle epoll_ctl()=EBADF/ENOENT in Schedule()
8ed533ac · Max Kellermann · a27580d0 · 8ed533ac · 8ed533ac
Commit 8ed533ac authored Apr 23, 2020 by Max Kellermann
Hide whitespace changes
Inline Side-by-side

Showing with 20 additions and 0 deletions

NEWS NEWS +1 -0

SocketMonitor.cxx src/event/SocketMonitor.cxx +19 -0

No files found.
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ ver 0.21.23 (not yet released)
  - curl: unescape "href" values
 * input
  - nfs: fix crash bug
+  - nfs: fix freeze bug on reconnect
 * decoder
  - gme: adapt to API change in the upcoming version 0.7.0
 * output

--- a/src/event/SocketMonitor.cxx
+++ b/src/event/SocketMonitor.cxx
@@ -20,6 +20,10 @@
 #include "SocketMonitor.hxx"
 #include "Loop.hxx"

+#ifdef USE_EPOLL
+#include <cerrno>
+#endif
+
 #include <assert.h>

 #ifdef _WIN32
@@ -86,6 +90,21 @@ SocketMonitor::Schedule(unsigned flags) noexcept

 	if (success)
 		scheduled_flags = flags;
+#ifdef USE_EPOLL
+	else if (errno == EBADF || errno == ENOENT)
+		/* the socket was probably closed by somebody else
+		   (EBADF) or a new file descriptor with the same
+		   number was created but not registered already
+		   (ENOENT) - we can assume that there are no
+		   scheduled events */
+		/* note that when this happens, we're actually lucky
+		   that it has failed - imagine another thread may
+		   meanwhile have created something on the same file
+		   descriptor number, and has registered it; the
+		   epoll_ctl() call above would then have succeeded,
+		   but broke the other thread's epoll registration */
+		scheduled_flags = 0;
+#endif

 	return success;
 }