[datalog] Guard FileLogger callbacks during log destruction

datalog/src/main/native/cpp/DataLog.cpp

@@ -9,6 +9,7 @@
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
+#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -35,6 +36,39 @@ static void DefaultLog(unsigned int level, const char* file, unsigned int line,
 
 wpi::util::Logger DataLog::s_defaultMessageLog{DefaultLog};
 
+struct DataLog::FileLoggerCallbackState {
+  explicit FileLoggerCallbackState(DataLog* log) : log{log} {}
+
+  wpi::util::mutex mutex;
+  DataLog* log;
+};
+
+DataLog::DataLog(wpi::util::Logger& msglog, std::string_view extraHeader)
+    : m_msglog{msglog},
+      m_extraHeader{extraHeader},
+      m_fileLoggerCallbackState{
+          std::make_shared<FileLoggerCallbackState>(this)} {}
+
+DataLog::~DataLog() {
+  InvalidateFileLoggerCallbacks();
+}
+
+void DataLog::InvalidateFileLoggerCallbacks() {
+  std::scoped_lock lock{m_fileLoggerCallbackState->mutex};
+  m_fileLoggerCallbackState->log = nullptr;
+}
+
+std::function<void(std::string_view)> DataLog::MakeFileLoggerCallback(
+    std::string_view key) {
+  int entry = Start(key, "string");
+  return [entry, state = m_fileLoggerCallbackState](std::string_view line) {
+    std::scoped_lock lock{state->mutex};
+    if (state->log) {
+      state->log->AppendString(entry, line, 0);
+    }
+  };
+}
+
 template <typename T>
 static unsigned int WriteVarInt(uint8_t* buf, T val) {
   unsigned int len = 0;

datalog/src/main/native/cpp/DataLogBackgroundWriter.cpp

@@ -81,6 +81,7 @@ DataLogBackgroundWriter::DataLogBackgroundWriter(
       }} {}
 
 DataLogBackgroundWriter::~DataLogBackgroundWriter() {
+  InvalidateFileLoggerCallbacks();
   {
     std::scoped_lock lock{m_mutex};
     m_shutdown = true;

datalog/src/main/native/cpp/DataLogWriter.cpp

@@ -47,6 +47,7 @@ DataLogWriter::DataLogWriter(wpi::util::Logger& msglog,
 }
 
 DataLogWriter::~DataLogWriter() {
+  InvalidateFileLoggerCallbacks();
   if (m_os) {
     Flush();
   }

datalog/src/main/native/cpp/FileLogger.cpp

@@ -69,10 +69,7 @@ FileLogger::FileLogger(std::string_view file,
 }
 FileLogger::FileLogger(std::string_view file, log::DataLog& log,
                        std::string_view key)
-    : FileLogger(file, Buffer([entry = log.Start(key, "string"),
-                               &log](std::string_view line) {
-                   log.AppendString(entry, line, 0);
-                 })) {}
+    : FileLogger(file, Buffer(log.MakeFileLoggerCallback(key))) {}
 FileLogger::FileLogger(FileLogger&& other)
 #ifdef __linux__
     : m_fileHandle{std::exchange(other.m_fileHandle, -1)},

datalog/src/main/native/include/wpi/datalog/DataLog.hpp

@@ -8,7 +8,9 @@
 
 #include <algorithm>
 #include <concepts>
+#include <functional>
 #include <initializer_list>
+#include <memory>
 #include <optional>
 #include <ranges>
 #include <span>
@@ -34,6 +36,8 @@ class Logger;
 
 namespace wpi::log {
 
+class FileLogger;
+
 namespace impl {
 
 enum ControlRecordType {
@@ -68,7 +72,7 @@ enum ControlRecordType {
  */
 class DataLog {
  public:
-  virtual ~DataLog() = default;
+  virtual ~DataLog();
 
   DataLog(const DataLog&) = delete;
   DataLog& operator=(const DataLog&) = delete;
@@ -441,8 +445,11 @@ class DataLog {
    * @param msglog message logger (will be called from separate thread)
    * @param extraHeader extra header metadata
    */
-  explicit DataLog(wpi::util::Logger& msglog, std::string_view extraHeader = "")
-      : m_msglog{msglog}, m_extraHeader{extraHeader} {}
+  explicit DataLog(wpi::util::Logger& msglog,
+                   std::string_view extraHeader = "");
+
+  /** Prevents FileLogger callbacks from accessing this log. */
+  void InvalidateFileLoggerCallbacks();
 
   /**
    * Starts the log.  Appends file header and Start records and schema data
@@ -487,6 +494,13 @@ class DataLog {
   virtual bool BufferFull() = 0;
 
  private:
+  friend class FileLogger;
+
+  struct FileLoggerCallbackState;
+
+  std::function<void(std::string_view)> MakeFileLoggerCallback(
+      std::string_view key);
+
   static constexpr size_t kMaxBufferCount = 1024 * 1024 / kBlockSize;
   static constexpr size_t kMaxFreeCount = 256 * 1024 / kBlockSize;
 
@@ -528,6 +542,7 @@ class DataLog {
   };
   wpi::util::DenseMap<int, EntryInfo2> m_entryIds;
   int m_lastId = 0;
+  std::shared_ptr<FileLoggerCallbackState> m_fileLoggerCallbackState;
 };
 
 /**

datalog/src/main/python/semiwrap/DataLog.yml

@@ -69,6 +69,8 @@ classes:
         ignore: true
       BufferHalfFull:
       BufferFull:
+      InvalidateFileLoggerCallbacks:
+        ignore: true
   wpi::log::DataLogEntry:
     force_no_trampoline: true
     methods:

datalog/src/test/native/cpp/FileLoggerTest.cpp

@@ -6,14 +6,20 @@
 
 #include <atomic>
 #include <chrono>
+#include <filesystem>
 #include <format>
+#include <fstream>
+#include <memory>
 #include <string>
 #include <string_view>
 #include <thread>
 #include <vector>
 
 #include <catch2/catch_test_macros.hpp>
 
+#include "wpi/datalog/DataLogWriter.hpp"
+#include "wpi/util/raw_ostream.hpp"
+
 #ifdef __linux__
 #include <fcntl.h>
 #include <unistd.h>
@@ -69,6 +75,31 @@ TEST_CASE("FileLoggerTest BufferMultipleMultiLinePartials",
 }
 
 #ifdef __linux__
+TEST_CASE("FileLoggerTest DataLogCanBeDestroyedFirst",
+          "[datalog][file-logger]") {
+  auto path = std::filesystem::temp_directory_path() /
+              std::format("wpi_filelogger_log_lifetime_{}", getpid());
+  {
+    std::ofstream create{path};
+  }
+
+  std::vector<uint8_t> output;
+  auto log = std::make_unique<wpi::log::DataLogWriter>(
+      std::make_unique<wpi::util::raw_uvector_ostream>(output));
+  wpi::log::FileLogger logger{path.string(), *log, "console"};
+
+  log.reset();
+  for (int i = 0; i < 100; ++i) {
+    {
+      std::ofstream append{path, std::ios::app};
+      append << "line\n";
+    }
+    std::this_thread::sleep_for(std::chrono::milliseconds{10});
+  }
+
+  std::filesystem::remove(path);
+}
+
 TEST_CASE("FileLoggerTest MissingFileDoesNotDeadlock",
           "[datalog][file-logger]") {
   // Constructing a FileLogger for a nonexistent path used to spawn a reader