memcheck: fix race in libgit2...

...caused by incorrectly setting and resetting the library internal
state and the misuse of pthreads in libgit2.

Normally, git_libgit2_init and git_libgit2_shutdown should span the
life of a worker thread in order to be safely used. However, due to
an incorrect implementation of libgit2's threadstate with pthreads,
on unix systems there is a race condition.

Until the use of pthread_key_t is corrected in libgit2, we need to
apply a workaround by always ensuring that the main thread is the
first thread reaching the GitContext constructor.

8 files changed, 38 insertions, 8 deletions

diff --git a/src/buildtool/file_system/TARGETS b/src/buildtool/file_system/TARGETS
index 8d2f524b..1af5dea8 100644
--- a/src/buildtool/file_system/TARGETS
+++ b/src/buildtool/file_system/TARGETS
@@ -85,7 +85,11 @@
   , "hdrs": ["git_context.hpp"]
   , "srcs": ["git_context.cpp"]
   , "stage": ["src", "buildtool", "file_system"]
-  , "private-deps": [["src/buildtool/logging", "logging"], ["", "libgit2"]]
+  , "private-deps":
+    [ ["src/buildtool/logging", "logging"]
+    , ["", "libgit2"]
+    , ["src/utils/cpp", "gsl"]
+    ]
   }
 , "git_repo":
   { "type": ["@", "rules", "CC", "library"]
diff --git a/src/buildtool/file_system/git_cas.cpp b/src/buildtool/file_system/git_cas.cpp
index 081d080f..f134a894 100644
--- a/src/buildtool/file_system/git_cas.cpp
+++ b/src/buildtool/file_system/git_cas.cpp
@@ -49,6 +49,10 @@ namespace {
 
 #endif  // BOOTSTRAP_BUILD_TOOL
 
+GitCAS::GitCAS() noexcept {
+    GitContext::Create();
+}
+
 auto GitCAS::Open(std::filesystem::path const& repo_path) noexcept
     -> GitCASPtr {
 #ifndef BOOTSTRAP_BUILD_TOOL
diff --git a/src/buildtool/file_system/git_cas.hpp b/src/buildtool/file_system/git_cas.hpp
index f95cb403..3ef8b6d8 100644
--- a/src/buildtool/file_system/git_cas.hpp
+++ b/src/buildtool/file_system/git_cas.hpp
@@ -34,7 +34,7 @@ class GitCAS {
     static auto Open(std::filesystem::path const& repo_path) noexcept
         -> GitCASPtr;
 
-    GitCAS() noexcept = default;
+    GitCAS() noexcept;
     ~GitCAS() noexcept = default;
 
     // prohibit moves and copies
@@ -62,8 +62,6 @@ class GitCAS {
         -> std::optional<std::pair<std::size_t, ObjectType>>;
 
   private:
-    // IMPORTANT: the GitContext needs to be initialized before any git object!
-    GitContext git_context_{};  // maintains a Git context while CAS is alive
     std::unique_ptr<git_odb, decltype(&odb_closer)> odb_{nullptr, odb_closer};
     // git folder path of repo
     std::filesystem::path git_path_{};
diff --git a/src/buildtool/file_system/git_context.cpp b/src/buildtool/file_system/git_context.cpp
index 04d4498d..084e9d58 100644
--- a/src/buildtool/file_system/git_context.cpp
+++ b/src/buildtool/file_system/git_context.cpp
@@ -15,6 +15,7 @@
 #include "src/buildtool/file_system/git_context.hpp"
 
 #include "src/buildtool/logging/logger.hpp"
+#include "src/utils/cpp/gsl.hpp"
 
 extern "C" {
 #include <git2.h>
@@ -34,4 +35,9 @@ GitContext::~GitContext() noexcept {
         git_libgit2_shutdown();
     }
 #endif
-}
\ No newline at end of file
+}
+
+void GitContext::Create() noexcept {
+    static GitContext git_state{};
+    Expects(git_state.initialized_);
+}
diff --git a/src/buildtool/file_system/git_context.hpp b/src/buildtool/file_system/git_context.hpp
index d6aaeab6..8cec02db 100644
--- a/src/buildtool/file_system/git_context.hpp
+++ b/src/buildtool/file_system/git_context.hpp
@@ -26,10 +26,11 @@ class GitContext {
     auto operator=(GitContext const&) = delete;
     auto operator=(GitContext&& other) = delete;
 
-    GitContext() noexcept;
+    static void Create() noexcept;
     ~GitContext() noexcept;
 
   private:
+    GitContext() noexcept;
     bool initialized_{false};
 };
 
diff --git a/src/buildtool/file_system/git_repo.cpp b/src/buildtool/file_system/git_repo.cpp
index d135875a..9fe63fb6 100644
--- a/src/buildtool/file_system/git_repo.cpp
+++ b/src/buildtool/file_system/git_repo.cpp
@@ -391,7 +391,7 @@ auto GitRepo::InitAndOpen(std::filesystem::path const& repo_path,
         static std::mutex repo_mutex{};
         std::unique_lock lock{repo_mutex};
 
-        auto git_state = GitContext();  // initialize libgit2
+        GitContext::Create();  // initialize libgit2
 
         // check if init is actually needed
         if (git_repository_open_ext(nullptr,
@@ -1015,7 +1015,7 @@ auto GitRepo::GetRepoRootFromPath(std::filesystem::path const& fpath,
     return std::nullopt;
 #else
     try {
-        auto git_state = GitContext();  // initialize libgit2
+        GitContext::Create();  // initialize libgit2
 
         git_buf buffer = GIT_BUF_INIT_CONST(NULL, 0);
         auto res = git_repository_discover(&buffer, fpath.c_str(), 0, nullptr);
diff --git a/src/buildtool/main/main.cpp b/src/buildtool/main/main.cpp
index 81a932a1..79c7c0cb 100644
--- a/src/buildtool/main/main.cpp
+++ b/src/buildtool/main/main.cpp
@@ -1264,7 +1264,16 @@ auto main(int argc, char* argv[]) -> int {
             Evaluator::SetExpressionLogLimit(
                 *arguments.analysis.expression_log_limit);
         }
+
 #ifndef BOOTSTRAP_BUILD_TOOL
+        /**
+         * The current implementation of libgit2 uses pthread_key_t incorrectly
+         * on POSIX systems to handle thread-specific data, which requires us to
+         * explicitly make sure the main thread is the first one to call
+         * git_libgit2_init. Future versions of libgit2 will hopefully fix this.
+         */
+        GitContext::Create();
+
         SetupHashFunction();
         SetupExecutionConfig(
             arguments.endpoint, arguments.build, arguments.rebuild);
diff --git a/src/other_tools/just_mr/main.cpp b/src/other_tools/just_mr/main.cpp
index e115e7e2..f47a5993 100644
--- a/src/other_tools/just_mr/main.cpp
+++ b/src/other_tools/just_mr/main.cpp
@@ -1481,6 +1481,14 @@ auto main(int argc, char* argv[]) -> int {
             return kExitGenericFailure;
         }
 
+        /**
+         * The current implementation of libgit2 uses pthread_key_t incorrectly
+         * on POSIX systems to handle thread-specific data, which requires us to
+         * explicitly make sure the main thread is the first one to call
+         * git_libgit2_init. Future versions of libgit2 will hopefully fix this.
+         */
+        GitContext::Create();
+
         // Run subcommands known to just and `do`
         if (arguments.cmd == SubCommand::kJustDo or
             arguments.cmd == SubCommand::kJustSubCmd) {