From 8a7329e0a8e680a1dca7c782f2dcc830fc75e4e6 Mon Sep 17 00:00:00 2001
From: Corey Kosak <kosak@users.noreply.github.com>
Date: Wed, 24 Jul 2024 19:40:43 -0400
Subject: [PATCH] feat(cpp-client): Add GetTidAsString as a utility function
 (#5843)

Add `GetTidAsString()` as a utility function.

Note that the previous version of this code was `inline`. However, on
the Windows platform, making it inline involves pulling in some windows
includes including `windows.h`. The problem with including `windows.h`
is that it defines some macros that might conflict. In particular it
defines the `min` and `max` macros that definitely conflict.

In order to free ourselves and our Windows customers from seeing the
conflicts that might arise, we isolate the Windows-specific code in a
.cc file rather than including `windows.h` in a .h file.

There are other approaches that would work too, such as defining
`NOMINMAX` or changing all of our calls that look like `min()` and
`max()` to have extra parentheses: `(min)()` and `(max)()`.

However in my opinion, isolating this Windows-specific code in one .cc
file was simplest for now.
---
 cpp-client/deephaven/dhcore/CMakeLists.txt    |  1 +
 .../public/deephaven/dhcore/utility/utility.h |  6 +++++
 .../src/utility/utility_platform_specific.cc  | 27 +++++++++++++++++++
 .../deephaven/tests/src/utility_test.cc       | 10 +++++++
 4 files changed, 44 insertions(+)
 create mode 100644 cpp-client/deephaven/dhcore/src/utility/utility_platform_specific.cc

diff --git a/cpp-client/deephaven/dhcore/CMakeLists.txt b/cpp-client/deephaven/dhcore/CMakeLists.txt
index 4209f7cc974..6186051d5f0 100644
--- a/cpp-client/deephaven/dhcore/CMakeLists.txt
+++ b/cpp-client/deephaven/dhcore/CMakeLists.txt
@@ -31,6 +31,7 @@ set(ALL_FILES
     src/ticking/ticking.cc
     src/utility/cython_support.cc
     src/utility/utility.cc
+    src/utility/utility_platform_specific.cc
 
     include/private/deephaven/dhcore/ticking/immer_table_state.h
     include/private/deephaven/dhcore/ticking/index_decoder.h
diff --git a/cpp-client/deephaven/dhcore/include/public/deephaven/dhcore/utility/utility.h b/cpp-client/deephaven/dhcore/include/public/deephaven/dhcore/utility/utility.h
index 4d177b0146b..354c3d18fbe 100644
--- a/cpp-client/deephaven/dhcore/include/public/deephaven/dhcore/utility/utility.h
+++ b/cpp-client/deephaven/dhcore/include/public/deephaven/dhcore/utility/utility.h
@@ -197,6 +197,12 @@ TimePointToStr(
  */
 std::string Basename(std::string_view path);
 
+/**
+ * Returns the current thread ID as a string.
+ * @return The current thread ID as a string.
+ */
+[[nodiscard]] std::string GetTidAsString();
+
 template <class T> [[nodiscard]] std::string
 TypeName(const T& t) {
   return demangle(typeid(t).name());
diff --git a/cpp-client/deephaven/dhcore/src/utility/utility_platform_specific.cc b/cpp-client/deephaven/dhcore/src/utility/utility_platform_specific.cc
new file mode 100644
index 00000000000..35e75d6c9d0
--- /dev/null
+++ b/cpp-client/deephaven/dhcore/src/utility/utility_platform_specific.cc
@@ -0,0 +1,27 @@
+/*
+ * Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
+ */
+#include <string>
+#include "deephaven/dhcore/utility/utility.h"
+
+// for GetTidAsString
+#if defined(__linux__)
+#include <unistd.h>
+#include <sys/syscall.h>
+#elif defined(_WIN32)
+#include <windows.h>
+#endif
+
+namespace deephaven::dhcore::utility {
+[[nodiscard]] std::string GetTidAsString() {
+#if defined(__linux__)
+  const pid_t tid = syscall(__NR_gettid);  // this is more portable than gettid().
+  return std::to_string(tid);
+#elif defined(_WIN32)
+  auto tid = GetCurrentThreadId();
+return std::to_string(tid);
+#else
+#error "Don't have a way to getting thread id on your platform"
+#endif
+}
+}  // namespace deephaven::dhcore::utility
diff --git a/cpp-client/deephaven/tests/src/utility_test.cc b/cpp-client/deephaven/tests/src/utility_test.cc
index 371494fd059..dd600761796 100644
--- a/cpp-client/deephaven/tests/src/utility_test.cc
+++ b/cpp-client/deephaven/tests/src/utility_test.cc
@@ -7,6 +7,7 @@
 using deephaven::dhcore::utility::Base64Encode;
 using deephaven::dhcore::utility::Basename;
 using deephaven::dhcore::utility::EpochMillisToStr;
+using deephaven::dhcore::utility::GetTidAsString;
 using deephaven::dhcore::utility::ObjectId;
 
 namespace deephaven::client::tests {
@@ -41,4 +42,13 @@ TEST_CASE("Basename", "[utility]") {
   CHECK(Basename(R"(C:\Users\kosak\)").empty());
 #endif
 }
+
+// This isn't much of a test, but if it can compile on all supported
+// platforms (Linux and Windows) then that is at least a sanity check
+// (that the entry point exists). For now we just visuallyi spot-check
+// that ireturns the right value.
+TEST_CASE("ThreadId", "[utility]") {
+  auto tid = GetTidAsString();
+  fmt::println("This should be my thread id: {}", tid);
+}
 }  // namespace deephaven::client::tests