plotfi created this revision.
plotfi added reviewers: jkorous, compnerd.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.
There are cases where the DirectoryWatcherTests just hang in a deadlock when
there are inotify limits hit, with no error reporting. This patch is a first
attempt to resolve these issues by bubbling up errors to the user.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65704
Files:
clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
clang/lib/DirectoryWatcher/default/DirectoryWatcher-not-implemented.cpp
clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===================================================================
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -280,6 +280,10 @@
TestConsumer.consume(Events, IsInitial);
},
/*waitForInitialSync=*/true);
+ if (!DW) {
+ llvm::errs() << llvm::toString(DW.takeError()) << "\n";
+ llvm_unreachable("DirectoryWatcher::create() FAILED!");
+ }
checkEventualResultWithTimeout(TestConsumer);
}
@@ -311,6 +315,10 @@
TestConsumer.consume(Events, IsInitial);
},
/*waitForInitialSync=*/false);
+ if (!DW) {
+ llvm::errs() << llvm::toString(DW.takeError()) << "\n";
+ llvm_unreachable("DirectoryWatcher::create() FAILED!");
+ }
checkEventualResultWithTimeout(TestConsumer);
}
@@ -331,6 +339,10 @@
TestConsumer.consume(Events, IsInitial);
},
/*waitForInitialSync=*/true);
+ if (!DW) {
+ llvm::errs() << llvm::toString(DW.takeError()) << "\n";
+ llvm_unreachable("DirectoryWatcher::create() FAILED!");
+ }
fixture.addFile("a");
fixture.addFile("b");
@@ -356,6 +368,10 @@
TestConsumer.consume(Events, IsInitial);
},
/*waitForInitialSync=*/true);
+ if (!DW) {
+ llvm::errs() << llvm::toString(DW.takeError()) << "\n";
+ llvm_unreachable("DirectoryWatcher::create() FAILED!");
+ }
// modify the file
{
@@ -386,6 +402,10 @@
TestConsumer.consume(Events, IsInitial);
},
/*waitForInitialSync=*/true);
+ if (!DW) {
+ llvm::errs() << llvm::toString(DW.takeError()) << "\n";
+ llvm_unreachable("DirectoryWatcher::create() FAILED!");
+ }
fixture.deleteFile("a");
@@ -407,6 +427,10 @@
TestConsumer.consume(Events, IsInitial);
},
/*waitForInitialSync=*/true);
+ if (!DW) {
+ llvm::errs() << llvm::toString(DW.takeError()) << "\n";
+ llvm_unreachable("DirectoryWatcher::create() FAILED!");
+ }
remove_directories(fixture.TestWatchedDir);
@@ -427,7 +451,11 @@
TestConsumer.consume(Events, IsInitial);
},
/*waitForInitialSync=*/true);
+ if (!DW) {
+ llvm::errs() << llvm::toString(DW.takeError()) << "\n";
+ llvm_unreachable("DirectoryWatcher::create() FAILED!");
+ }
} // DW is destructed here.
checkEventualResultWithTimeout(TestConsumer);
}
\ No newline at end of file
Index: clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
===================================================================
--- clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
+++ clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
@@ -11,6 +11,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/Path.h"
#include <CoreServices/CoreServices.h>
@@ -205,7 +206,7 @@
FSEventStreamRelease(EventStream);
}
-std::unique_ptr<DirectoryWatcher> clang::DirectoryWatcher::create(
+llvm::Expected<std::unique_ptr<DirectoryWatcher>> clang::DirectoryWatcher::create(
StringRef Path,
std::function<void(llvm::ArrayRef<DirectoryWatcher::Event>, bool)> Receiver,
bool WaitForInitialSync) {
@@ -213,12 +214,15 @@
dispatch_queue_create("DirectoryWatcher", DISPATCH_QUEUE_SERIAL);
if (Path.empty())
- return nullptr;
+ return llvm::make_error<llvm::StringError>(
+ std::string("Path.empty() error: ") + strerror(errno),
+ llvm::inconvertibleErrorCode());
auto EventStream = createFSEventStream(Path, Receiver, Queue);
- if (!EventStream) {
- return nullptr;
- }
+ if (!EventStream)
+ return llvm::make_error<llvm::StringError>(
+ std::string("createFSEventStream() error: ") + strerror(errno),
+ llvm::inconvertibleErrorCode());
std::unique_ptr<DirectoryWatcher> Result =
llvm::make_unique<DirectoryWatcherMac>(EventStream, Receiver, Path);
Index: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
===================================================================
--- clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
+++ clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
@@ -13,6 +13,7 @@
#include "llvm/ADT/ScopeExit.h"
#include "llvm/Support/AlignOf.h"
#include "llvm/Support/Errno.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/Mutex.h"
#include "llvm/Support/Path.h"
#include <atomic>
@@ -28,6 +29,7 @@
#include <sys/epoll.h>
#include <sys/inotify.h>
#include <unistd.h>
+#include <iostream>
namespace {
@@ -321,16 +323,20 @@
} // namespace
-std::unique_ptr<DirectoryWatcher> clang::DirectoryWatcher::create(
+llvm::Expected<std::unique_ptr<DirectoryWatcher>> clang::DirectoryWatcher::create(
StringRef Path,
std::function<void(llvm::ArrayRef<DirectoryWatcher::Event>, bool)> Receiver,
bool WaitForInitialSync) {
if (Path.empty())
- return nullptr;
+ return llvm::make_error<llvm::StringError>(
+ std::string("Path.empty() error: ") + strerror(errno),
+ llvm::inconvertibleErrorCode());
const int InotifyFD = inotify_init1(IN_CLOEXEC);
if (InotifyFD == -1)
- return nullptr;
+ return llvm::make_error<llvm::StringError>(
+ std::string("inotify_init1() error: ") + strerror(errno),
+ llvm::inconvertibleErrorCode());
const int InotifyWD = inotify_add_watch(
InotifyFD, Path.str().c_str(),
@@ -341,12 +347,16 @@
#endif
);
if (InotifyWD == -1)
- return nullptr;
+ return llvm::make_error<llvm::StringError>(
+ std::string("inotify_add_watch() error: ") + strerror(errno),
+ llvm::inconvertibleErrorCode();
auto InotifyPollingStopper = SemaphorePipe::create();
if (!InotifyPollingStopper)
- return nullptr;
+ return llvm::make_error<llvm::StringError>(
+ std::string("SemaphorePipe::create() error: ") + strerror(errno),
+ llvm::inconvertibleErrorCode());
return llvm::make_unique<DirectoryWatcherLinux>(
Path, Receiver, WaitForInitialSync, InotifyFD, InotifyWD,
Index: clang/lib/DirectoryWatcher/default/DirectoryWatcher-not-implemented.cpp
===================================================================
--- clang/lib/DirectoryWatcher/default/DirectoryWatcher-not-implemented.cpp
+++ clang/lib/DirectoryWatcher/default/DirectoryWatcher-not-implemented.cpp
@@ -11,9 +11,11 @@
using namespace llvm;
using namespace clang;
-std::unique_ptr<DirectoryWatcher> clang::DirectoryWatcher::create(
+llvm::Expected<std::unique_ptr<DirectoryWatcher>> clang::DirectoryWatcher::create(
StringRef Path,
std::function<void(llvm::ArrayRef<DirectoryWatcher::Event>, bool)> Receiver,
bool WaitForInitialSync) {
- return nullptr;
+ return llvm::make_error<llvm::StringError>(
+ "DirectoryWatcher is not implemented for this platform!",
+ llvm::inconvertibleErrorCode());
}
\ No newline at end of file
Index: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
===================================================================
--- clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
+++ clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
@@ -11,6 +11,7 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Error.h"
#include <functional>
#include <memory>
#include <string>
@@ -101,7 +102,7 @@
/// Returns nullptr if \param Path doesn't exist or isn't a directory.
/// Returns nullptr if OS kernel API told us we can't start watching. In such
/// case it's unclear whether just retrying has any chance to succeeed.
- static std::unique_ptr<DirectoryWatcher>
+ static llvm::Expected<std::unique_ptr<DirectoryWatcher>>
create(llvm::StringRef Path,
std::function<void(llvm::ArrayRef<DirectoryWatcher::Event> Events,
bool IsInitial)>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
