Protect CheckManager with mutex

Needed in case Clazy is used as a plugin inside libclang. There, we
might have multiple threads accessing CheckManager concurrently.

4 files changed, 28 insertions, 1 deletions

diff --git a/src/Clazy.cpp b/src/Clazy.cpp
index 0b1875ab..6b9ecc94 100644
--- a/src/Clazy.cpp
+++ b/src/Clazy.cpp
@@ -37,6 +37,7 @@
 #include "clang/AST/ParentMap.h"
 #include <llvm/Config/llvm-config.h>
 
+#include <mutex>
 #include <stdio.h>
 #include <sstream>
 #include <iostream>
@@ -153,6 +154,11 @@ ClazyASTAction::ClazyASTAction()
 
 std::unique_ptr<clang::ASTConsumer> ClazyASTAction::CreateASTConsumer(CompilerInstance &, llvm::StringRef)
 {
+    // NOTE: This method needs to be kept reentrant (but not necessarily thread-safe)
+    // Might be called from multiple threads via libclang, each thread operates on a different instance though
+
+    std::lock_guard<std::mutex> lock(CheckManager::lock());
+
     auto astConsumer = std::unique_ptr<ClazyASTConsumer>(new ClazyASTConsumer(m_context));
     CheckBase::List createdChecks = m_checkManager->createChecks(m_checks, m_context);
     for (CheckBase *check : createdChecks) {
@@ -164,6 +170,9 @@ std::unique_ptr<clang::ASTConsumer> ClazyASTAction::CreateASTConsumer(CompilerIn
 
 bool ClazyASTAction::ParseArgs(const CompilerInstance &ci, const std::vector<std::string> &args_)
 {
+    // NOTE: This method needs to be kept reentrant (but not necessarily thread-safe)
+    // Might be called from multiple threads via libclang, each thread operates on a different instance though
+
     std::vector<std::string> args = args_;
 
     if (parseArgument("help", args)) {
@@ -208,7 +217,10 @@ bool ClazyASTAction::ParseArgs(const CompilerInstance &ci, const std::vector<std
     // This argument is for debugging purposes
     const bool dbgPrintRequestedChecks = parseArgument("print-requested-checks", args);
 
-    m_checks = m_checkManager->requestedChecks(m_context, args);
+    {
+        std::lock_guard<std::mutex> lock(CheckManager::lock());
+        m_checks = m_checkManager->requestedChecks(m_context, args);
+    }
 
     if (args.size() > 1) {
         // Too many arguments.
@@ -277,7 +289,9 @@ void ClazyASTAction::PrintAnchorHeader(llvm::raw_ostream &ros, RegisteredCheck::
 
 void ClazyASTAction::PrintHelp(llvm::raw_ostream &ros, HelpMode helpMode) const
 {
+    std::lock_guard<std::mutex> lock(CheckManager::lock());
     RegisteredCheck::List checks = m_checkManager->availableChecks(MaxCheckLevel);
+
     clazy_std::sort(checks, checkLessThanByLevel);
 
     if (helpMode == HelpMode_AnchorHeader) {
diff --git a/src/Clazy.h b/src/Clazy.h
index 8d89d893..2c7c3b75 100644
--- a/src/Clazy.h
+++ b/src/Clazy.h
@@ -55,8 +55,11 @@ public:
     ClazyASTAction();
 
 protected:
+    /// @note This function is reentrant
     std::unique_ptr<clang::ASTConsumer> CreateASTConsumer(clang::CompilerInstance &ci, llvm::StringRef) override;
+    /// @note This function is reentrant
     bool ParseArgs(const clang::CompilerInstance &ci, const std::vector<std::string> &args_) override;
+
     void PrintHelp(llvm::raw_ostream &ros, HelpMode = HelpMode_Normal) const;
     void PrintAnchorHeader(llvm::raw_ostream &ro, RegisteredCheck::List &checks) const;
 private:
diff --git a/src/checkmanager.cpp b/src/checkmanager.cpp
index a9080809..534e6371 100644
--- a/src/checkmanager.cpp
+++ b/src/checkmanager.cpp
@@ -36,6 +36,8 @@ using namespace std;
 static const char * s_fixitNamePrefix = "fix-";
 static const char * s_levelPrefix = "level";
 
+std::mutex CheckManager::m_lock;
+
 CheckManager::CheckManager()
 {
     m_registeredChecks.reserve(100);
diff --git a/src/checkmanager.h b/src/checkmanager.h
index 63e00a7a..efedd26d 100644
--- a/src/checkmanager.h
+++ b/src/checkmanager.h
@@ -77,8 +77,15 @@ inline bool checkLessThanByLevel(const RegisteredCheck &c1, const RegisteredChec
 class CLAZYLIB_EXPORT CheckManager
 {
 public:
+    /**
+     * @note You must hold the CheckManager lock when operating on the instance
+     *
+     * @sa lock()
+     */
     static CheckManager *instance();
 
+    static std::mutex &lock() { return m_lock; }
+
     int registerCheck(const std::string &name, const std::string &className,
                       CheckLevel level, const FactoryFunction &, RegisteredCheck::Options = RegisteredCheck::Option_None);
     int registerFixIt(int id, const std::string &fititName, const std::string &checkName);
@@ -105,6 +112,7 @@ public:
 
 private:
     CheckManager();
+    static std::mutex m_lock;
 
     bool checkExists(const std::string &name) const;
     RegisteredCheck::List checksForLevel(int level) const;