From 3db69cdae68013666758c8215328f48480793c9d Mon Sep 17 00:00:00 2001
From: Mikael Simberg <mikael.simberg@iki.fi>
Date: Tue, 14 Jan 2025 16:14:22 +0100
Subject: [PATCH] Patch gtest so that it terminates if a ScopedTrace is
 destroyed on thread different than where it was destroyed

gtest ScopedTrace/SCOPED_TRACE uses thread local stacks to keep track of traces. When combined with pika
tasks that may yield and be stolen onto other worker threads, destroying a ScopedTrace on a different
thread can segfault, cause hangs, or simply corrupt data. Instead of allowing that to happen every now
and then we instead patch gtest to check if this happens and terminate as soon as it's detected. Use of
ScopedTrace should be avoided in situations where tasks can yield. This patch does not fix the issue, but
helps recognizing when e.g. a segfault is the cause of "only" improper use of ScopedTrace or if it is a
result of other algorithmic or implementation issues.
---
 external/CMakeLists.txt                       |  4 +-
 ...ed_trace_terminate_on_thread_migrate.patch | 76 +++++++++++++++++++
 2 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 external/scoped_trace_terminate_on_thread_migrate.patch

diff --git a/external/CMakeLists.txt b/external/CMakeLists.txt
index 9cb2b1fcf6..cb429de82b 100644
--- a/external/CMakeLists.txt
+++ b/external/CMakeLists.txt
@@ -18,7 +18,9 @@ FetchContent_Declare(
   GIT_REPOSITORY https://github.com/google/googletest.git
   GIT_TAG v1.13.0
   # Patch for int to float conversion warning.
-  PATCH_COMMAND git cherry-pick -n 5cd81a7848203d3df6959c148eb91f1a4ff32c1d -m 1
+  PATCH_COMMAND
+    git reset --hard HEAD && git cherry-pick -n 5cd81a7848203d3df6959c148eb91f1a4ff32c1d -m 1 && git
+    apply "${CMAKE_CURRENT_SOURCE_DIR}/scoped_trace_terminate_on_thread_migrate.patch"
 )
 set(BUILD_GMOCK OFF CACHE INTERNAL "")
 set(INSTALL_GTEST OFF CACHE INTERNAL "")
diff --git a/external/scoped_trace_terminate_on_thread_migrate.patch b/external/scoped_trace_terminate_on_thread_migrate.patch
new file mode 100644
index 0000000000..ad39c80e78
--- /dev/null
+++ b/external/scoped_trace_terminate_on_thread_migrate.patch
@@ -0,0 +1,76 @@
+diff --git a/googletest/include/gtest/gtest.h b/googletest/include/gtest/gtest.h
+index 3e452a50..c51dd607 100644
+--- a/googletest/include/gtest/gtest.h
++++ b/googletest/include/gtest/gtest.h
+@@ -58,6 +58,7 @@
+ #include <set>
+ #include <sstream>
+ #include <string>
++#include <thread>
+ #include <type_traits>
+ #include <vector>
+ 
+@@ -2056,15 +2057,18 @@ class GTEST_API_ ScopedTrace {
+   // Slow, but flexible.
+   template <typename T>
+   ScopedTrace(const char* file, int line, const T& message) {
++    id = std::this_thread::get_id();
+     PushTrace(file, line, (Message() << message).GetString());
+   }
+ 
+   // Optimize for some known types.
+   ScopedTrace(const char* file, int line, const char* message) {
++    id = std::this_thread::get_id();
+     PushTrace(file, line, message ? message : "(null)");
+   }
+ 
+   ScopedTrace(const char* file, int line, const std::string& message) {
++    id = std::this_thread::get_id();
+     PushTrace(file, line, message);
+   }
+ 
+@@ -2079,6 +2083,8 @@ class GTEST_API_ ScopedTrace {
+ 
+   ScopedTrace(const ScopedTrace&) = delete;
+   ScopedTrace& operator=(const ScopedTrace&) = delete;
++
++  std::thread::id id;
+ };
+ 
+ // Causes a trace (including the source file path, the current line
+diff --git a/googletest/src/gtest.cc b/googletest/src/gtest.cc
+index a64e887c..2e1fccb9 100644
+--- a/googletest/src/gtest.cc
++++ b/googletest/src/gtest.cc
+@@ -44,14 +44,17 @@
+ #include <chrono>  // NOLINT
+ #include <cmath>
+ #include <cstdint>
++#include <exception>
+ #include <initializer_list>
+ #include <iomanip>
++#include <iostream>
+ #include <iterator>
+ #include <limits>
+ #include <list>
+ #include <map>
+ #include <ostream>  // NOLINT
+ #include <sstream>
++#include <thread>
+ #include <unordered_set>
+ #include <vector>
+ 
+@@ -6838,6 +6841,13 @@ void ScopedTrace::PushTrace(const char* file, int line, std::string message) {
+ 
+ // Pops the info pushed by the c'tor.
+ ScopedTrace::~ScopedTrace() GTEST_LOCK_EXCLUDED_(&UnitTest::mutex_) {
++  if (id != std::this_thread::get_id()) {
++    std::cerr << "ScopedTrace was created and destroyed on different "
++                 "std::threads, terminating to avoid segfaults, hangs, or "
++                 "silent corruption. Are you using any pika functionality that "
++                 "may yield a task after creating the ScopedTrace?\n";
++    std::terminate();
++  }
+   UnitTest::GetInstance()->PopGTestTrace();
+ }
+