From 9593acc73cacffff585353063392ffc0200c6e2d Mon Sep 17 00:00:00 2001
From: Peter Johnson <johnson.peter@gmail.com>
Date: Sat, 23 Dec 2023 11:38:24 -0800
Subject: [PATCH] [wpiutil] DataLog: Ensure file is written on shutdown

Previously the thread could end without the file being written.
---
 wpiutil/src/main/native/cpp/DataLog.cpp       | 10 +++++-----
 wpiutil/src/main/native/include/wpi/DataLog.h |  2 +-
 wpiutil/src/test/native/cpp/DataLogTest.cpp   | 18 ++++++++++++++++++
 3 files changed, 24 insertions(+), 6 deletions(-)
 create mode 100644 wpiutil/src/test/native/cpp/DataLogTest.cpp

diff --git a/wpiutil/src/main/native/cpp/DataLog.cpp b/wpiutil/src/main/native/cpp/DataLog.cpp
index d05a49e43b7..fa0938dd74b 100644
--- a/wpiutil/src/main/native/cpp/DataLog.cpp
+++ b/wpiutil/src/main/native/cpp/DataLog.cpp
@@ -179,7 +179,7 @@ DataLog::DataLog(wpi::Logger& msglog,
 DataLog::~DataLog() {
   {
     std::scoped_lock lock{m_mutex};
-    m_state = kShutdown;
+    m_shutdown = true;
     m_doFlush = true;
   }
   m_cond.notify_all();
@@ -419,7 +419,7 @@ void DataLog::WriterThreadMain(std::string_view dir) {
   uintmax_t written = 0;
 
   std::unique_lock lock{m_mutex};
-  while (m_state != kShutdown) {
+  do {
     bool doFlush = false;
     auto timeoutTime = std::chrono::steady_clock::now() + periodTime;
     if (m_cond.wait_until(lock, timeoutTime) == std::cv_status::timeout) {
@@ -557,7 +557,7 @@ void DataLog::WriterThreadMain(std::string_view dir) {
       }
       toWrite.resize(0);
     }
-  }
+  } while (!m_shutdown);
 }
 
 void DataLog::WriterThreadMain(
@@ -580,7 +580,7 @@ void DataLog::WriterThreadMain(
   std::vector<Buffer> toWrite;
 
   std::unique_lock lock{m_mutex};
-  while (m_state != kShutdown) {
+  do {
     bool doFlush = false;
     auto timeoutTime = std::chrono::steady_clock::now() + periodTime;
     if (m_cond.wait_until(lock, timeoutTime) == std::cv_status::timeout) {
@@ -614,7 +614,7 @@ void DataLog::WriterThreadMain(
       }
       toWrite.resize(0);
     }
-  }
+  } while (!m_shutdown);
 
   write({});  // indicate EOF
 }
diff --git a/wpiutil/src/main/native/include/wpi/DataLog.h b/wpiutil/src/main/native/include/wpi/DataLog.h
index c78deafce16..86633699499 100644
--- a/wpiutil/src/main/native/include/wpi/DataLog.h
+++ b/wpiutil/src/main/native/include/wpi/DataLog.h
@@ -486,12 +486,12 @@ class DataLog final {
   mutable wpi::mutex m_mutex;
   wpi::condition_variable m_cond;
   bool m_doFlush{false};
+  bool m_shutdown{false};
   enum State {
     kStart,
     kActive,
     kPaused,
     kStopped,
-    kShutdown,
   } m_state = kActive;
   double m_period;
   std::string m_extraHeader;
diff --git a/wpiutil/src/test/native/cpp/DataLogTest.cpp b/wpiutil/src/test/native/cpp/DataLogTest.cpp
new file mode 100644
index 00000000000..a098f373276
--- /dev/null
+++ b/wpiutil/src/test/native/cpp/DataLogTest.cpp
@@ -0,0 +1,18 @@
+// Copyright (c) FIRST and other WPILib contributors.
+// Open Source Software; you can modify and/or share it under the terms of
+// the WPILib BSD license file in the root directory of this project.
+
+#include "wpi/DataLog.h"
+
+#include <gtest/gtest.h>
+
+TEST(DataLog, SimpleInt) {
+  std::vector<uint8_t> data;
+  {
+    wpi::log::DataLog log{
+        [&](auto out) { data.insert(data.end(), out.begin(), out.end()); }};
+    int entry = log.Start("test", "int64");
+    log.AppendInteger(entry, 1, 0);
+  }
+  ASSERT_EQ(data.size(), 66u);
+}