chore: ThreadPool returns errors

Author: vinniefalco

include/mrdox/Support/Error.hpp

@@ -26,7 +26,7 @@ namespace mrdox {
 /** Holds the description of an error, or success.
 */
 class [[nodiscard]] MRDOX_DECL
-    Error : public std::exception
+    Error final : public std::exception
 {
     std::string text_;
 

include/mrdox/Support/Report.hpp

@@ -81,11 +81,19 @@ reportError(
 }
 
 template<class Range>
-void
+Error
 reportErrors(Range const& errors)
 {
+    MRDOX_ASSERT(std::begin(errors) != std::end(errors));
+    std::size_t n = 0;
     for(auto const& err : errors)
+    {
+        ++n;
         reportError(err.message());
+    }
+    if(n > 1)
+        return Error("{} errors occurred", n);
+    return Error("an error occurred");
 }
 
 /** Report a warning to the console.

include/mrdox/Support/ThreadPool.hpp

@@ -13,11 +13,12 @@
 #define MRDOX_SUPPORT_THREAD_HPP
 
 #include <mrdox/Platform.hpp>
-#include <mrdox/Support/Error.hpp>
 #include <mrdox/Support/any_callable.hpp>
+#include <mrdox/Support/Error.hpp>
 #include <memory>
 #include <type_traits>
 #include <utility>
+#include <vector>
 
 namespace llvm {
 class ThreadPool;
@@ -41,10 +42,10 @@ class MRDOX_VISIBLE
     friend class TaskGroup;
 
 public:
-    template<class Agent> struct arg_ty { using type = Agent; };
-    template<class Agent> struct arg_ty<Agent&> { using type =
-        std::conditional_t< std::is_const_v<Agent>, Agent, Agent&>; };
-    template<class Agent> using arg_t = typename arg_ty<Agent>::type;
+    template<class T> struct arg_ty { using type = T; };
+    template<class T> struct arg_ty<T&> { using type =
+        std::conditional_t< std::is_const_v<T>, T, T&>; };
+    template<class T> using arg_t = typename arg_ty<T>::type;
 
     /** Destructor.
     */
@@ -93,9 +94,14 @@ class MRDOX_VISIBLE
     }
 
     /** Invoke a function object for each element of a range.
+
+        @return Zero or more errors which were
+        thrown from submitted work.
     */
     template<class Range, class F>
-    void forEach(Range&& range, F const& f);
+    [[nodiscard]]
+    std::vector<Error>
+    forEach(Range&& range, F const& f);
 
     /** Block until all work has completed.
     */
@@ -114,7 +120,9 @@ class MRDOX_VISIBLE
 class MRDOX_VISIBLE
     TaskGroup
 {
-    std::unique_ptr<llvm::ThreadPoolTaskGroup> impl_;
+    struct Impl;
+
+    std::unique_ptr<Impl> impl_;
 
 public:
     /** Destructor.
@@ -142,9 +150,13 @@ class MRDOX_VISIBLE
     }
 
     /** Block until all work has completed.
+
+        @return Zero or more errors which were
+        thrown from submitted work.
     */
     MRDOX_DECL
-    void
+    [[nodiscard]]
+    std::vector<Error>
     wait();
 
 private:
@@ -154,7 +166,7 @@ class MRDOX_VISIBLE
 //------------------------------------------------
 
 template<class Range, class F>
-void
+std::vector<Error>
 ThreadPool::
 forEach(
     Range&& range,
@@ -167,7 +179,7 @@ forEach(
             {
                 f(value);
             });
-    taskGroup.wait();
+    return taskGroup.wait();
 }
 
 } // mrdox

source/-adoc/AdocGenerator.cpp

@@ -83,10 +83,7 @@ buildOne(
     visitor(corpus.globalNamespace());
     auto errors = ex->wait();
     if(! errors.empty())
-    {
-        reportErrors(errors);
-        return Error("one or more errors occurred");
-    }
+        return reportErrors(errors);
     return Error::success();
 }
 

source/-bitcode/BitcodeGenerator.cpp

@@ -43,8 +43,10 @@ class MultiFileBuilder : public Corpus::Visitor
     build()
     {
         corpus_.traverse(*this, SymbolID::zero);
-        taskGroup_.wait();
-        return Error();
+        auto errors = taskGroup_.wait();
+        if(! errors.empty())
+            return reportErrors(errors);
+        return Error::success();
     }
 
     template<class T>

source/Support/Thread.cpp renamed to source/Support/ThreadPool.cpp

@@ -11,6 +11,8 @@
 #include "Support/Debug.hpp"
 #include <mrdox/Support/ThreadPool.hpp>
 #include <llvm/Support/ThreadPool.h>
+#include <mutex>
+#include <unordered_set>
 #include <utility>
 
 namespace clang {
@@ -82,6 +84,22 @@ post(
 //
 //------------------------------------------------
 
+struct TaskGroup::
+    Impl
+{
+    std::mutex mutex;
+    std::unordered_set<Error> errors;
+    llvm::ThreadPoolTaskGroup taskGroup;
+
+    explicit
+    Impl(
+        llvm::ThreadPool& threadPool)
+        : taskGroup(threadPool)
+    {
+    }
+};
+
+
 TaskGroup::
 ~TaskGroup()
 {
@@ -90,15 +108,30 @@ TaskGroup::
 TaskGroup::
 TaskGroup(
     ThreadPool& threadPool)
-    : impl_(std::make_unique<llvm::ThreadPoolTaskGroup>(*threadPool.impl_))
+    : impl_(std::make_unique<Impl>(*threadPool.impl_))
 {
 }
 
-void
+std::vector<Error>
 TaskGroup::
 wait()
 {
-    impl_->wait();
+    impl_->taskGroup.wait();
+
+    // VFALCO We could have a small data race here
+    // where another thread posts work after the
+    // wait is satisfied, but that could be
+    // considered user error.
+    //
+    // In theory the lock should not be needed.
+    //
+    std::lock_guard<std::mutex> lock(impl_->mutex);
+    std::vector<Error> errors;
+    errors.reserve(impl_->errors.size());
+    for(auto& err : impl_->errors)
+        errors.emplace_back(std::move(err));
+    impl_->errors.clear();
+    return errors;
 }
 
 void
@@ -108,10 +141,22 @@ post(
 {
     auto sp = std::make_shared<
         any_callable<void(void)>>(std::move(f));
-    impl_->async(
-        [sp]
+    impl_->taskGroup.async(
+        [&, sp]
         {
-            (*sp)();
+            try
+            {
+                (*sp)();
+            }
+            catch(Error const& err)
+            {
+                std::lock_guard<std::mutex> lock(impl_->mutex);
+                impl_->errors.emplace(std::move(err));
+            }
+            // Any exception which is not
+            // derived from Error should
+            // be reported and terminate
+            // the process immediately.
         });
 }
 

source/Tool/CorpusImpl.cpp

@@ -144,7 +144,7 @@ build(
         reportInfo("Reducing {} declarations", bitcodes.size());
     std::atomic<bool> GotFailure;
     GotFailure = false;
-    corpus->config.threadPool().forEach(
+    auto errors = corpus->config.threadPool().forEach(
         bitcodes,
         [&](auto& Group)
         {
@@ -179,6 +179,8 @@ build(
             MRDOX_ASSERT(Group.getKey() == StringRef(I->id));
             corpus->insert(std::move(I));
         });
+    if(! errors.empty())
+        return reportErrors(errors);
 
     if(corpus->config.verboseOutput)
         llvm::outs() << "Collected " << corpus->InfoMap.size() << " symbols.\n";