Make FileLogger thread-safe

src/AppInstallerCLITests/FileLogger.cpp

@@ -4,6 +4,8 @@
 #include "TestCommon.h"
 #include <AppInstallerFileLogger.h>
 #include <AppInstallerStrings.h>
+#include <future>
+#include <set>
 
 using namespace AppInstaller::Logging;
 using namespace AppInstaller::Utility;
@@ -243,3 +245,79 @@ TEST_CASE("FileLogger_MaximumSize_ManyWraps", "[logging]")
 
     ValidateFileContents(tempFile, expectedFileContents, maximumSize);
 }
+
+TEST_CASE("FileLogger_ConcurrentWritesPreserveCompleteLines", "[logging]")
+{
+    bool formatted = GENERATE(false, true);
+    std::ofstream::off_type maximumSize = GENERATE(0, 4 * 1024 * 1024);
+    TempFile tempFile{ "FileLogger_ConcurrentWrites", ".log" };
+    constexpr size_t workerCount = 8;
+    constexpr size_t messagesPerWorker = 1000;
+    std::vector<std::vector<std::string>> messages(workerCount);
+    std::set<std::string> expected;
+    for (size_t worker = 0; worker < workerCount; ++worker)
+    {
+        for (size_t message = 0; message < messagesPerWorker; ++message)
+        {
+            auto text = "parallel-log:" + std::to_string(worker) + ":" + std::to_string(message) + ":" + std::string(128, 'x');
+            expected.insert(text);
+            messages[worker].emplace_back(std::move(text));
+        }
+    }
+
+    {
+        FileLogger original{ tempFile };
+        original.SetMaximumSize(maximumSize);
+        FileLogger logger{ std::move(original) };
+        std::promise<void> start;
+        auto started = start.get_future().share();
+        std::vector<std::future<void>> workers;
+        auto releaseOnFailure = wil::scope_exit([&]() { start.set_value(); });
+        for (size_t worker = 0; worker < workerCount; ++worker)
+        {
+            workers.emplace_back(std::async(std::launch::async, [&, worker]()
+                {
+                    started.wait();
+                    for (const auto& message : messages[worker])
+                    {
+                        if (formatted)
+                        {
+                            logger.Write(DefaultChannel, DefaultLevel, message);
+                        }
+                        else
+                        {
+                            logger.WriteDirect(DefaultChannel, DefaultLevel, message);
+                        }
+                    }
+                }));
+        }
+        start.set_value();
+        releaseOnFailure.release();
+        for (auto& worker : workers)
+        {
+            worker.get();
+        }
+    }
+
+    std::ifstream file{ tempFile.GetPath() };
+    REQUIRE(file.is_open());
+    std::string line;
+    size_t lineCount = 0;
+    while (std::getline(file, line))
+    {
+        INFO("Line " << lineCount << ": " << line);
+        std::string message = line;
+        if (formatted)
+        {
+            constexpr std::string_view prefix = " <I> [CORE] ";
+            auto position = line.find(prefix);
+            REQUIRE(position != std::string::npos);
+            message = line.substr(position + prefix.size());
+        }
+        REQUIRE(expected.erase(message) == 1);
+        ++lineCount;
+    }
+    REQUIRE(file.eof());
+    REQUIRE(lineCount == workerCount * messagesPerWorker);
+    REQUIRE(expected.empty());
+}

src/AppInstallerCommonCore/FileLogger.cpp

@@ -67,6 +67,7 @@ namespace AppInstaller::Logging
     FileLogger& FileLogger::SetMaximumSize(std::ofstream::off_type maximumSize)
     {
         THROW_HR_IF(E_INVALIDARG, maximumSize < 0);
+        std::lock_guard<std::mutex> lock{ *m_streamLock };
         m_maximumSize = maximumSize;
         return *this;
     }
@@ -101,6 +102,7 @@ namespace AppInstaller::Logging
 
     void FileLogger::WriteDirect(Channel, Level, std::string_view message) noexcept try
     {
+        std::lock_guard<std::mutex> lock{ *m_streamLock };
         HandleMaximumFileSize(message);
         m_stream << message << std::endl;
     }
@@ -110,6 +112,7 @@ namespace AppInstaller::Logging
     {
         if (tag == Tag::HeadersComplete)
         {
+            std::lock_guard<std::mutex> lock{ *m_streamLock };
             auto currentPosition = m_stream.tellp();
             if (currentPosition != std::ofstream::pos_type{ -1 })
             {

src/AppInstallerCommonCore/Public/AppInstallerFileLogger.h

@@ -5,6 +5,8 @@
 
 #include <filesystem>
 #include <fstream>
+#include <memory>
+#include <mutex>
 #include <string>
 #include <string_view>
 
@@ -55,6 +57,8 @@ namespace AppInstaller::Logging
     private:
         std::string m_name;
         std::filesystem::path m_filePath;
+        // Keep the stream lock movable with the logger.
+        std::unique_ptr<std::mutex> m_streamLock = std::make_unique<std::mutex>();
         std::ofstream m_stream;
         std::ofstream::pos_type m_headersEnd = 0;
         std::ofstream::off_type m_maximumSize = 0;