Skip to content

Commit

Permalink
Patch gtest to terminate as soon as it detects that a ScopedTrace h…
Browse files Browse the repository at this point in the history
…as migrated to another worker thread (#1258)
  • Loading branch information
msimberg authored Jan 29, 2025
1 parent a61bdc6 commit f112e64
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .github/format.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ do
# Check if tab are present.
egrep -Hn $'\t' $FILE && TAB_FOUND=1 || true
;;
*.pdf|*.hdf5|*.jpg|*.png|*.ppt|*.pptx|*.ipe)
*.pdf|*.hdf5|*.jpg|*.patch|*.png|*.ppt|*.pptx|*.ipe)
# Exclude some binary files types,
# others can be excluded as needed.
;;
Expand Down
4 changes: 3 additions & 1 deletion external/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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 "")
Expand Down
76 changes: 76 additions & 0 deletions external/scoped_trace_terminate_on_thread_migrate.patch
Original file line number Diff line number Diff line change
@@ -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();
}

0 comments on commit f112e64

Please sign in to comment.