event/InotifyEvent: new class wrapping inotify

Replaces class InotifySource.

src/db/meson.build

@@ -43,7 +43,6 @@ db_glue_sources = [
 if enable_inotify
   db_glue_sources += [
     'update/InotifyDomain.cxx',
-    'update/InotifySource.cxx',
     'update/InotifyQueue.cxx',
     'update/InotifyUpdate.cxx',
   ]

src/db/update/InotifySource.cxx

@@ -1,115 +0,0 @@
-/*
- * Copyright 2003-2021 The Music Player Daemon Project
- * http://www.musicpd.org
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-#include "InotifySource.hxx"
-#include "InotifyDomain.hxx"
-#include "io/FileDescriptor.hxx"
-#include "system/Error.hxx"
-#include "Log.hxx"
-
-#include <cerrno>
-#include <climits>
-#include <cstdint>
-
-#include <sys/inotify.h>
-
-void
-InotifySource::OnSocketReady([[maybe_unused]] unsigned flags) noexcept
-{
-	uint8_t buffer[4096];
-	static_assert(sizeof(buffer) >= sizeof(struct inotify_event) + NAME_MAX + 1,
-		      "inotify buffer too small");
-
-	auto ifd = socket_event.GetFileDescriptor();
-	ssize_t nbytes = ifd.Read(buffer, sizeof(buffer));
-	if (nbytes <= 0) {
-		if (nbytes < 0)
-			FmtError(inotify_domain,
-				 "Failed to read from inotify: {}",
-				 strerror(errno));
-		else
-			LogError(inotify_domain,
-				 "end of file from inotify");
-		socket_event.Cancel();
-		return;
-	}
-
-	const uint8_t *p = buffer, *const end = p + nbytes;
-
-	while (true) {
-		const size_t remaining = end - p;
-		const auto *event =
-			(const struct inotify_event *)p;
-		if (remaining < sizeof(*event) ||
-		    remaining < sizeof(*event) + event->len)
-			break;
-
-		const char *name;
-		if (event->len > 0 && event->name[event->len - 1] == 0)
-			name = event->name;
-		else
-			name = nullptr;
-
-		callback(event->wd, event->mask, name, callback_ctx);
-		p += sizeof(*event) + event->len;
-	}
-}
-
-static FileDescriptor
-InotifyInit()
-{
-	FileDescriptor fd;
-	if (!fd.CreateInotify())
-		throw MakeErrno("inotify_init() has failed");
-
-	return fd;
-}
-
-InotifySource::InotifySource(EventLoop &_loop,
-			     mpd_inotify_callback_t _callback, void *_ctx)
-	:socket_event(_loop, BIND_THIS_METHOD(OnSocketReady),
-		      InotifyInit()),
-	 callback(_callback), callback_ctx(_ctx)
-{
-	socket_event.ScheduleRead();
-}
-
-int
-InotifySource::Add(const char *path_fs, unsigned mask)
-{
-	auto ifd = socket_event.GetFileDescriptor();
-	int wd = inotify_add_watch(ifd.Get(), path_fs, mask);
-	if (wd < 0)
-		throw MakeErrno("inotify_add_watch() has failed");
-
-	return wd;
-}
-
-void
-InotifySource::Remove(unsigned wd) noexcept
-{
-	auto ifd = socket_event.GetFileDescriptor();
-	int ret = inotify_rm_watch(ifd.Get(), wd);
-	if (ret < 0 && errno != EINVAL)
-		FmtError(inotify_domain, "inotify_rm_watch() has failed: {}",
-			 strerror(errno));
-
-	/* EINVAL may happen here when the file has been deleted; the
-	   kernel seems to auto-unregister deleted files */
-}

src/db/update/InotifySource.hxx

@@ -1,71 +0,0 @@
-/*
- * Copyright 2003-2021 The Music Player Daemon Project
- * http://www.musicpd.org
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
- */
-
-#ifndef MPD_INOTIFY_SOURCE_HXX
-#define MPD_INOTIFY_SOURCE_HXX
-
-#include "event/PipeEvent.hxx"
-
-typedef void (*mpd_inotify_callback_t)(int wd, unsigned mask,
-				       const char *name, void *ctx);
-
-class InotifySource final {
-	PipeEvent socket_event;
-
-	mpd_inotify_callback_t callback;
-	void *callback_ctx;
-
-public:
-	/**
-	 * Creates a new inotify source and registers it in the
-	 * #EventLoop.
-	 *
-	 * Throws #std::system_error on error.
-	 *
-	 * @param callback a callback invoked for events received from
-	 * the kernel
-	 */
-	InotifySource(EventLoop &_loop,
-		      mpd_inotify_callback_t callback, void *ctx);
-
-	~InotifySource() noexcept {
-		socket_event.Close();
-	}
-
-	/**
-	 * Adds a path to the notify list.
-	 *
-	 * Throws #std::system_error on error.
-	 *
-	 * @return a watch descriptor
-	 */
-	int Add(const char *path_fs, unsigned mask);
-
-	/**
-	 * Removes a path from the notify list.
-	 *
-	 * @param wd the watch descriptor returned by mpd_inotify_source_add()
-	 */
-	void Remove(unsigned wd) noexcept;
-
-private:
-	void OnSocketReady(unsigned flags) noexcept;
-};
-
-#endif

src/db/update/InotifyUpdate.cxx

@@ -118,7 +118,7 @@ InotifyUpdate::Disable(WatchDirectory &directory) noexcept
 	for (WatchDirectory &child : directory.children)
 		Disable(child);
 
-	source.Remove(directory.descriptor);
+	inotify_event.RemoveWatch(directory.descriptor);
 }
 
 void
@@ -199,7 +199,8 @@ try {
 			continue;
 
 		try {
-			ret = source.Add(child_path_fs.c_str(), IN_MASK);
+			ret = inotify_event.AddWatch(child_path_fs.c_str(),
+						     IN_MASK);
 		} catch (...) {
 			FmtError(inotify_domain,
 				 "Failed to register {}: {}",
@@ -240,7 +241,7 @@ WatchDirectory::GetDepth() const noexcept
 inline
 InotifyUpdate::InotifyUpdate(EventLoop &loop, UpdateService &update,
 			     unsigned _max_depth)
-	:source(loop, InotifyCallback, this),
+	:inotify_event(loop, *this),
 	 queue(loop, update),
 	 max_depth(_max_depth)
 {
@@ -251,7 +252,7 @@ InotifyUpdate::~InotifyUpdate() noexcept = default;
 inline void
 InotifyUpdate::Start(Path path)
 {
-	int descriptor = source.Add(path.c_str(), IN_MASK);
+	int descriptor = inotify_event.AddWatch(path.c_str(), IN_MASK);
 
 	root = std::make_unique<WatchDirectory>(path, descriptor);
 	root->LoadExcludeList(path);
@@ -262,8 +263,7 @@ InotifyUpdate::Start(Path path)
 }
 
 void
-InotifyUpdate::InotifyCallback(int wd, unsigned mask,
+InotifyUpdate::OnInotify(int wd, unsigned mask, const char *)
-			       [[maybe_unused]] const char *name) noexcept
 {
 	auto i = directories.find(wd);
 	if (i == directories.end())
@@ -310,6 +310,12 @@ InotifyUpdate::InotifyCallback(int wd, unsigned mask,
 	}
 }
 
+void
+InotifyUpdate::OnInotifyError(std::exception_ptr error) noexcept
+{
+	LogError(error, "inotify error");
+}
+
 std::unique_ptr<InotifyUpdate>
 mpd_inotify_init(EventLoop &loop, Storage &storage, UpdateService &update,
 		 unsigned max_depth)

src/db/update/InotifyUpdate.hxx

@@ -20,8 +20,8 @@
 #ifndef MPD_INOTIFY_UPDATE_HXX
 #define MPD_INOTIFY_UPDATE_HXX
 
-#include "InotifySource.hxx"
 #include "InotifyQueue.hxx"
+#include "event/InotifyEvent.hxx"
 
 #include <map>
 #include <memory>
@@ -33,8 +33,8 @@ struct WatchDirectory;
 /**
  * Glue code between InotifySource and InotifyQueue.
  */
-class InotifyUpdate {
+class InotifyUpdate final : InotifyHandler {
-	InotifySource source;
+	InotifyEvent inotify_event;
 	InotifyQueue queue;
 
 	const unsigned max_depth;
@@ -50,14 +50,6 @@ public:
 	void Start(Path path);
 
 private:
-	void InotifyCallback(int wd, unsigned mask, const char *name) noexcept;
-
-	static void InotifyCallback(int wd, unsigned mask,
-				    const char *name, void *ctx) noexcept {
-		auto &iu = *(InotifyUpdate *)ctx;
-		iu.InotifyCallback(wd, mask, name);
-	}
-
 	void AddToMap(WatchDirectory &directory) noexcept;
 	void RemoveFromMap(WatchDirectory &directory) noexcept;
 	void Disable(WatchDirectory &directory) noexcept;
@@ -66,6 +58,11 @@ private:
 	void RecursiveWatchSubdirectories(WatchDirectory &parent,
 					  Path path_fs,
 					  unsigned depth) noexcept;
+
+private:
+	/* virtual methods from class InotifyHandler */
+	void OnInotify(int wd, unsigned mask, const char *name) override;
+	void OnInotifyError(std::exception_ptr error) noexcept override;
 };
 
 /**

src/event/InotifyEvent.cxx

@@ -0,0 +1,129 @@
+/*
+ * Copyright 2022 CM4all GmbH
+ * All rights reserved.
+ *
+ * author: Max Kellermann <mk@cm4all.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE
+ * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "InotifyEvent.hxx"
+#include "system/Error.hxx"
+#include "io/UniqueFileDescriptor.hxx"
+
+#include <array>
+
+#include <limits.h>
+#include <sys/inotify.h>
+
+static UniqueFileDescriptor
+CreateInotify()
+{
+	int fd = inotify_init1(IN_CLOEXEC|IN_NONBLOCK);
+	if (fd < 0)
+		throw MakeErrno("inotify_init1() failed");
+
+	return UniqueFileDescriptor(fd);
+}
+
+InotifyEvent::InotifyEvent(EventLoop &event_loop, InotifyHandler &_handler)
+	:event(event_loop, BIND_THIS_METHOD(OnInotifyReady),
+	       CreateInotify().Release()),
+	 handler(_handler)
+{
+	Enable();
+}
+
+InotifyEvent::~InotifyEvent() noexcept
+{
+	Close();
+}
+
+int
+InotifyEvent::AddWatch(const char *pathname, uint32_t mask)
+{
+	int wd = inotify_add_watch(event.GetFileDescriptor().Get(),
+				   pathname, mask);
+	if (wd < 0)
+		throw FormatErrno("inotify_add_watch('%s') failed", pathname);
+
+	return wd;
+}
+
+int
+InotifyEvent::AddModifyWatch(const char *pathname)
+{
+	return AddWatch(pathname, IN_MODIFY);
+}
+
+void
+InotifyEvent::RemoveWatch(int wd) noexcept
+{
+	inotify_rm_watch(event.GetFileDescriptor().Get(), wd);
+}
+
+inline void
+InotifyEvent::OnInotifyReady(unsigned) noexcept
+try {
+	std::array<std::byte, 4096> buffer;
+	static_assert(sizeof(buffer) >= sizeof(struct inotify_event) + NAME_MAX + 1,
+		      "inotify buffer too small");
+
+	ssize_t nbytes = event.GetFileDescriptor().Read(buffer.data(),
+							buffer.size());
+	if (nbytes <= 0) [[unlikely]] {
+		if (nbytes == 0)
+			throw std::runtime_error{"EOF from inotify"};
+
+		const int e = errno;
+		if (e == EAGAIN)
+			return;
+
+		throw MakeErrno(e, "Reading inotify failed");
+	}
+
+	const std::byte *p = buffer.data(), *const end = p + nbytes;
+
+	while (true) {
+		const size_t remaining = end - p;
+		const auto &ie = *(const struct inotify_event *)(const void *)p;
+		if (remaining < sizeof(ie) ||
+		    remaining < sizeof(ie) + ie.len)
+			break;
+
+		const char *name;
+		if (ie.len > 0 && ie.name[ie.len - 1] == 0)
+			name = ie.name;
+		else
+			name = nullptr;
+
+		handler.OnInotify(ie.wd, ie.mask, name);
+		p += sizeof(ie) + ie.len;
+	}
+} catch (...) {
+	Close();
+	handler.OnInotifyError(std::current_exception());
+}

src/event/InotifyEvent.hxx

@@ -0,0 +1,129 @@
+/*
+ * Copyright 2022 CM4all GmbH
+ * All rights reserved.
+ *
+ * author: Max Kellermann <mk@cm4all.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE
+ * FOUNDATION OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include "PipeEvent.hxx"
+
+#include <exception>
+
+/**
+ * Handler for #InotifyEvent.
+ */
+class InotifyHandler {
+public:
+	/**
+	 * An inotify event was received.
+	 *
+	 * @param wd the watch descriptor returned by
+	 * InotifyEvent::AddWatch().
+	 */
+	virtual void OnInotify(int wd, unsigned mask, const char *name) = 0;
+
+	/**
+	 * An (permanent) inotify error has occurred, and the
+	 * #InotifyEvent has been closed.
+	 */
+	virtual void OnInotifyError(std::exception_ptr error) noexcept = 0;
+};
+
+/**
+ * #EventLoop integration for Linux inotify.
+ */
+class InotifyEvent final {
+	PipeEvent event;
+
+	InotifyHandler &handler;
+
+public:
+	/**
+	 * Create an inotify file descriptor add register it in the
+	 * #EventLoop.
+	 *
+	 * Throws on error.
+	 */
+	InotifyEvent(EventLoop &event_loop, InotifyHandler &_handler);
+
+	~InotifyEvent() noexcept;
+
+	EventLoop &GetEventLoop() const noexcept {
+		return event.GetEventLoop();
+	}
+
+	/**
+	 * Re-enable polling the inotify file descriptor after it was
+	 * disabled by Disable().
+	 */
+	void Enable() noexcept {
+		event.ScheduleRead();
+	}
+
+	/**
+	 * Disable polling the inotify file descriptor.  Can be
+	 * re-enabled by Enable().
+	 */
+	void Disable() noexcept {
+		event.Cancel();
+	}
+
+	/**
+	 * Permanently close the inotify file descriptor.  Further
+	 * method calls not allowed after that.
+	 */
+	void Close() noexcept {
+		event.Close();
+	}
+
+	/**
+	 * Register a new path to be watched.
+	 *
+	 * Throws on error.
+	 *
+	 * @return a watch descriptor
+	 */
+	int AddWatch(const char *pathname, uint32_t mask);
+
+	/**
+	 * Wrapper for AddWatch(pathname, IN_MODIFY).
+	 */
+	int AddModifyWatch(const char *pathname);
+
+	/**
+	 * Stop watching the given watch descriptor.
+	 *
+	 * @param wd a watch descriptor returned by AddWatch()
+	 */
+	void RemoveWatch(int wd) noexcept;
+
+private:
+	void OnInotifyReady(unsigned) noexcept;
+};

src/event/meson.build

@@ -19,6 +19,10 @@ else
   event_sources += 'PollBackend.cxx'
 endif
 
+if enable_inotify
+  event_sources += 'InotifyEvent.cxx'
+endif
+
 event = static_library(
   'event',
   'SignalMonitor.cxx',

src/io/FileDescriptor.cxx

@@ -44,7 +44,6 @@
 #ifdef __linux__
 #include <sys/eventfd.h>
 #include <sys/signalfd.h>
-#include <sys/inotify.h>
 #endif
 
 #ifndef O_NOCTTY
@@ -283,17 +282,6 @@ FileDescriptor::CreateSignalFD(const sigset_t *mask) noexcept
 	return true;
 }
 
-bool
-FileDescriptor::CreateInotify() noexcept
-{
-	int new_fd = inotify_init1(IN_CLOEXEC|IN_NONBLOCK);
-	if (new_fd < 0)
-		return false;
-
-	fd = new_fd;
-	return true;
-}
-
 #endif
 
 bool

src/io/FileDescriptor.hxx

@@ -209,7 +209,6 @@ public:
 #ifdef __linux__
 	bool CreateEventFD(unsigned initval=0) noexcept;
 	bool CreateSignalFD(const sigset_t *mask) noexcept;
-	bool CreateInotify() noexcept;
 #endif
 
 	/**

test/meson.build

@@ -133,8 +133,6 @@ if enable_inotify
     'run_inotify',
     'run_inotify.cxx',
     'ShutdownHandler.cxx',
-    '../src/db/update/InotifyDomain.cxx',
-    '../src/db/update/InotifySource.cxx',
     include_directories: inc,
     dependencies: [
       log_dep,

test/run_inotify.cxx

@@ -18,7 +18,7 @@
  */
 
 #include "ShutdownHandler.hxx"
-#include "db/update/InotifySource.hxx"
+#include "event/InotifyEvent.hxx"
 #include "event/Loop.hxx"
 #include "Log.hxx"
 
@@ -33,18 +33,23 @@ static constexpr unsigned IN_MASK =
 	IN_ATTRIB|IN_CLOSE_WRITE|IN_CREATE|IN_DELETE|IN_DELETE_SELF
 	|IN_MOVE|IN_MOVE_SELF;
 
-static void
+struct Instance final : InotifyHandler {
-my_inotify_callback([[maybe_unused]] int wd, unsigned mask,
-		    const char *name, [[maybe_unused]] void *ctx)
-{
-	printf("mask=0x%x name='%s'\n", mask, name);
-}
-
-struct Instance {
 	EventLoop event_loop;
 	const ShutdownHandler shutdown_handler{event_loop};
 
-	InotifySource source{event_loop, my_inotify_callback, nullptr};
+	InotifyEvent inotify_event{event_loop, *this};
+
+	std::exception_ptr error;
+
+	/* virtual methods from class InotifyHandler */
+	void OnInotify(int, unsigned mask, const char *name) override {
+		printf("mask=0x%x name='%s'\n", mask, name);
+	}
+
+	void OnInotifyError(std::exception_ptr _error) noexcept override {
+		error = std::move(_error);
+		event_loop.Break();
+	}
 };
 
 int main(int argc, char **argv)
@@ -60,10 +65,13 @@ try {
 
 	Instance instance;
 
-	instance.source.Add(path, IN_MASK);
+	instance.inotify_event.AddWatch(path, IN_MASK);
 
 	instance.event_loop.Run();
 
+	if (instance.error)
+		std::rethrow_exception(instance.error);
+
 	return EXIT_SUCCESS;
 } catch (...) {
 	LogError(std::current_exception());