FsWatch: more reliable construction even when facing errors

Author: Lewin Bormann

CMakeLists.txt

@@ -9,7 +9,7 @@ pkg_check_modules(FMT REQUIRED fmt)
 pkg_check_modules(UV REQUIRED libuv)
 
 # Allow using a custom libuv installation.
-link_directories(BEFORE /usr/local/lib64)
+link_directories(BEFORE /usr/local/lib64 /usr/local/lib)
 
 find_package(Boost 1.80 COMPONENTS log log_setup program_options REQUIRED)
 find_package(CURL)

test/fs_test.cc

@@ -10,6 +10,7 @@
 #include "uvco/loop/loop.h"
 #include "uvco/promise/multipromise.h"
 #include "uvco/promise/promise.h"
+#include "uvco/run.h"
 
 #include <algorithm>
 #include <cstddef>
@@ -161,7 +162,7 @@ TEST(FsWatchTest, basicFileWatch) {
   auto setup = [](const Loop &loop) -> Promise<void> {
     auto file = co_await File::open(loop, filename, O_RDWR | O_CREAT);
 
-    auto watch = FsWatch::create(loop, filename);
+    auto watch = co_await FsWatch::create(loop, filename);
     MultiPromise<FsWatch::FileEvent> watcher = watch.watch();
 
     co_await file.write("Hello World\n");
@@ -195,7 +196,7 @@ TEST(FsWatchTest, basicDirWatch) {
 
     auto file = co_await File::open(loop, filename, O_RDWR | O_CREAT);
 
-    auto watch = FsWatch::create(loop, dirName);
+    auto watch = co_await FsWatch::create(loop, dirName);
     MultiPromise<FsWatch::FileEvent> watcher = watch.watch();
 
     // Check that FsWatch::create is not recursive
@@ -235,7 +236,7 @@ TEST(DISABLED_FsWatchTest, watchRecursive) {
     co_await Directory::mkdir(loop, dirName);
     co_await Directory::mkdir(loop, subdirName);
 
-    auto watch = FsWatch::createRecursive(loop, dirName);
+    auto watch = co_await FsWatch::createRecursive(loop, dirName);
     MultiPromise<FsWatch::FileEvent> watcher = watch.watch();
 
     auto file = co_await File::open(loop, filename, O_RDWR | O_CREAT);
@@ -264,11 +265,12 @@ TEST(FsWatchTest, watchNonExisting) {
 
   auto setup = [](const Loop &loop) -> Promise<void> {
     try {
-      auto watcher = FsWatch::create(loop, filename);
+      auto watcher = co_await FsWatch::create(loop, filename);
       EXPECT_FALSE(true) << "Expected UvcoException";
     } catch (const UvcoException &e) {
       EXPECT_EQ(e.status, UV_ENOENT);
     }
+    co_await yield();
     co_return;
   };
 
@@ -281,7 +283,7 @@ TEST(FsWatchTest, repeatedWatchFails) {
   auto setup = [](const Loop &loop) -> Promise<void> {
     auto file = co_await File::open(loop, filename, O_RDWR | O_CREAT);
 
-    auto watch = FsWatch::create(loop, filename);
+    auto watch = co_await FsWatch::create(loop, filename);
     MultiPromise<FsWatch::FileEvent> watcher = watch.watch();
 
     try {

uvco/fs.cc

@@ -280,66 +280,80 @@ Promise<void> File::close() {
   co_await awaiter;
 }
 
+FsWatch::FsWatch() : uv_handle_{std::make_unique<uv_fs_event_t>()} {}
+
 FsWatch::~FsWatch() {
-  BOOST_ASSERT_MSG(dataIsNull(&uv_handle_),
+  if (!uv_handle_) {
+    return;
+  }
+  BOOST_ASSERT_MSG(dataIsNull(uv_handle_.get()),
                    "don't drop an FsWatch while a watch() generator is "
                    "running (use stopWatch()!)");
-  uv_fs_event_stop(&uv_handle_);
+  uv_fs_event_stop(uv_handle_.get());
+}
+
+Promise<FsWatch> FsWatch::create(const Loop &loop, std::string_view path) {
+  return createWithFlag(loop, path, {});
 }
 
-FsWatch FsWatch::create(const Loop &loop, std::string_view path) {
-  return FsWatch{loop, path, {}};
+Promise<FsWatch> FsWatch::createRecursive(const Loop &loop,
+                                          std::string_view path) {
+  return createWithFlag(loop, path, UV_FS_EVENT_RECURSIVE);
 }
 
-FsWatch FsWatch::createRecursive(const Loop &loop, std::string_view path) {
-  return FsWatch{loop, path, UV_FS_EVENT_RECURSIVE};
+Promise<FsWatch> FsWatch::createWithFlag(const Loop &loop,
+                                         std::string_view path,
+                                         uv_fs_event_flags flags) {
+  FsWatch fsWatch;
+  uv_fs_event_t &uv_handle = *fsWatch.uv_handle_;
+  const uv_status initStatus = uv_fs_event_init(loop.uvloop(), &uv_handle);
+  if (initStatus != 0) {
+    throw UvcoException{
+        initStatus,
+        "uv_fs_event_init returned error while initializing FsWatch"};
+  }
+  const auto startStatus =
+      callWithNullTerminated<uv_status>(path, [&](std::string_view safePath) {
+        return uv_fs_event_start(&uv_handle, onFsWatcherEvent, safePath.data(),
+                                 flags);
+      });
+  if (startStatus != 0) {
+    uv_fs_event_stop(&uv_handle);
+    // This works with the current Loop::~Loop implementation.
+    co_await closeHandle(&uv_handle);
+    fsWatch.uv_handle_.reset();
+    throw UvcoException{
+        startStatus, "uv_fs_event_start returned error while starting FsWatch"};
+  }
+  co_return fsWatch;
 }
 
 MultiPromise<FsWatch::FileEvent> FsWatch::watch() {
-  if (!dataIsNull(&uv_handle_)) {
+  if (!dataIsNull(uv_handle_.get())) {
     throw UvcoException(UV_EBUSY, "FsWatch::watch() is already active!");
   }
   return watch_();
 }
 
 MultiPromise<FsWatch::FileEvent> FsWatch::watch_() {
   FsWatchAwaiter_ awaiter;
-  setData(&uv_handle_, &awaiter);
-  ZeroAtExit<void> zeroAtExit{&uv_handle_.data};
+  setData(uv_handle_.get(), &awaiter);
+  ZeroAtExit<void> zeroAtExit{&uv_handle_->data};
 
   while (co_await awaiter) {
     co_yield awaiter.events_.get();
   }
 }
 
 Promise<void> FsWatch::stopWatch(MultiPromise<FsWatch::FileEvent> watcher) {
-  auto *awaiter = getData<FsWatchAwaiter_>(&uv_handle_);
+  auto *awaiter = getData<FsWatchAwaiter_>(uv_handle_.get());
   awaiter->stop();
   co_await watcher;
 }
 
-Promise<void> FsWatch::close() { return closeHandle(&uv_handle_); }
-
-FsWatch::FsWatch(const Loop &loop, std::string_view path,
-                 uv_fs_event_flags flags) {
-  const uv_status initStatus = uv_fs_event_init(loop.uvloop(), &uv_handle_);
-  if (initStatus != 0) {
-    throw UvcoException{
-        initStatus,
-        "uv_fs_event_init returned error while initializing FsWatch"};
-  }
-  const auto startStatus =
-      callWithNullTerminated<uv_status>(path, [&](std::string_view safePath) {
-        return uv_fs_event_start(&uv_handle_, onFsWatcherEvent, safePath.data(),
-                                 flags);
-      });
-  if (startStatus != 0) {
-    uv_fs_event_stop(&uv_handle_);
-    // This works with the current Loop::~Loop implementation.
-    uv_close((uv_handle_t *)&uv_handle_, nullptr);
-    throw UvcoException{
-        startStatus, "uv_fs_event_start returned error while starting FsWatch"};
-  }
+Promise<void> FsWatch::close() {
+  co_await closeHandle(uv_handle_.get());
+  uv_handle_.reset();
 }
 
 void FsWatch::onFsWatcherEvent(uv_fs_event_t *handle, const char *path,

uvco/fs.h

@@ -2,6 +2,7 @@
 
 #pragma once
 
+#include <memory>
 #include <uv.h>
 #include <uv/unix.h>
 
@@ -103,17 +104,24 @@ class File {
 
 class FsWatch {
 public:
+  FsWatch(const FsWatch &) = delete;
+  FsWatch(FsWatch &&) = default;
+  FsWatch &operator=(const FsWatch &) = delete;
+  FsWatch &operator=(FsWatch &&) = default;
+
+  /// Destructor. If the watch is still active, it is stopped. However, it's
+  /// still required to call close() before dropping the FsWatch.
+  ~FsWatch();
+
   /// Create aa new FsWatch instance. The path is the file or directory to
   /// watch.
-  static FsWatch create(const Loop &loop, std::string_view path);
+  static Promise<FsWatch> create(const Loop &loop, std::string_view path);
 
   /// Create a recursive watch. NOTE! This is only supported on macOS and
   /// Windows; see
   /// https://docs.libuv.org/en/v1.x/fs_event.html#c.uv_fs_event_start.
-  static FsWatch createRecursive(const Loop &loop, std::string_view path);
-
-  /// Destructor. If the watch is still active, it is stopped.
-  ~FsWatch();
+  static Promise<FsWatch> createRecursive(const Loop &loop,
+                                          std::string_view path);
 
   /// An event in an observed file. If status is not 0, an error occurred; this
   /// doesn't mean however that no more events occur.
@@ -151,7 +159,11 @@ class FsWatch {
   Promise<void> close();
 
 private:
-  FsWatch(const Loop &loop, std::string_view path, uv_fs_event_flags flags);
+  static Promise<FsWatch> createWithFlag(const Loop &loop,
+                                         std::string_view path,
+                                         uv_fs_event_flags flags);
+  FsWatch();
+
   MultiPromise<FileEvent> watch_();
   static void onFsWatcherEvent(uv_fs_event_t *handle, const char *path,
                                int events, uv_status status);
@@ -175,7 +187,7 @@ class FsWatch {
     bool stopped_{};
   };
 
-  uv_fs_event_t uv_handle_{};
+  std::unique_ptr<uv_fs_event_t> uv_handle_{};
 };
 
 /// @}