feat(cpp-client): Add entry points for SetEnv() and UnsetEnv() (deeph…

…aven#5987)

cpp-client/deephaven/dhcore/CMakeLists.txt

@@ -23,6 +23,7 @@ set(ALL_FILES
  src/immerutil/immer_column_source.cc
  src/interop/testapi/basic_interop_interactions.cc
  src/interop/interop_util.cc
+ src/interop/utility_interop.cc
  src/ticking/barrage_processor.cc
  src/ticking/immer_table_state.cc
  src/ticking/index_decoder.cc
@@ -52,6 +53,7 @@ set(ALL_FILES
  include/public/deephaven/dhcore/container/row_sequence.h
  include/public/deephaven/dhcore/interop/testapi/basic_interop_interactions.h
  include/public/deephaven/dhcore/interop/interop_util.h
+ include/public/deephaven/dhcore/interop/utility_interop.h
  include/public/deephaven/dhcore/ticking/barrage_processor.h
  include/public/deephaven/dhcore/ticking/ticking.h
  include/public/deephaven/dhcore/utility/cython_support.h

cpp-client/deephaven/dhcore/include/public/deephaven/dhcore/interop/utility_interop.h

@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
+ */
+#pragma once
+
+#include "deephaven/dhcore/interop/interop_util.h"
+
+extern "C" {
+void deephaven_dhcore_utility_GetEnv(const char *envname,
+ deephaven::dhcore::interop::StringHandle *string_handle,
+ deephaven::dhcore::interop::StringPoolHandle *string_pool_handle,
+ deephaven::dhcore::interop::ErrorStatus *status);
+
+void deephaven_dhcore_utility_SetEnv(const char *envname, const char *value,
+ deephaven::dhcore::interop::ErrorStatus *status);
+
+void deephaven_dhcore_utility_UnsetEnv(const char *envname,
+ deephaven::dhcore::interop::ErrorStatus *status);
+} // extern "C"

cpp-client/deephaven/dhcore/include/public/deephaven/dhcore/utility/utility.h

@@ -217,6 +217,19 @@ std::string Basename(std::string_view path);
  */
 [[nodiscard]] std::optional<std::string> GetEnv(const std::string& envname);
 
+/**
+ * Sets a value in the environment.
+ * @param envname the key
+ * @param value the value to set in the environment
+ */
+void SetEnv(const std::string& envname, const std::string& value);
+
+/**
+ * Unsets a value in the environment.
+ * @param envname the key to unset
+ */
+void UnsetEnv(const std::string& envname);
+
 /**
  * Enables or disables echo for stdin.
  * @param enable true to enable, false to disable

cpp-client/deephaven/dhcore/src/interop/utility_interop.cc

@@ -0,0 +1,42 @@
+/*
+ * Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
+ */
+#include "deephaven/dhcore/interop/utility_interop.h"
+
+#include "deephaven/dhcore/interop/interop_util.h"
+#include "deephaven/dhcore/utility/utility.h"
+
+using deephaven::dhcore::interop::ErrorStatus;
+using deephaven::dhcore::interop::StringHandle;
+using deephaven::dhcore::interop::StringPoolBuilder;
+using deephaven::dhcore::interop::StringPoolHandle;
+using deephaven::dhcore::utility::GetEnv;
+using deephaven::dhcore::utility::SetEnv;
+using deephaven::dhcore::utility::UnsetEnv;
+
+extern "C" {
+void deephaven_dhcore_utility_GetEnv(const char *envname,
+ StringHandle *string_handle, StringPoolHandle *string_pool_handle, ErrorStatus *status) {
+ status->Run([=]() {
+ StringPoolBuilder builder;
+ auto result = GetEnv(envname);
+ if (result.has_value()) {
+ *string_handle = builder.Add(*result);
+ }
+ // If envname is not found in the environment, caller will see an empty builder
+ *string_pool_handle = builder.Build();
+ });
+}
+
+void deephaven_dhcore_utility_SetEnv(const char *envname, const char *value, ErrorStatus *status) {
+ status->Run([=]() {
+ SetEnv(envname, value);
+ });
+}
+
+void deephaven_dhcore_utility_UnsetEnv(const char *envname, ErrorStatus *status) {
+ status->Run([=]() {
+ UnsetEnv(envname);
+ });
+}
+} // extern "C"

cpp-client/deephaven/dhcore/src/utility/utility_platform_specific.cc

@@ -104,21 +104,99 @@ std::string GetHostname() {
 #endif
 }
 
+namespace {
+/**
+ * This method wraps a function-local static mutex. The purpose of this mutex is to
+ * synchronize the calls GetEnv(), SetEnv(), and UnsetEnv().
+ *
+ * The rationale for synchronizing these calls is that they are not guaranteed
+ * reentrant on either Linux or Windows. On Linux, "man getenv" says
+ *
+ * The implementation of getenv() is not required to be reentrant. The
+ * string pointed to by the return value of getenv() may be statically al‐
+ * located, and can be modified by a subsequent call to getenv(),
+ * putenv(3), setenv(3), or unsetenv(3).
+ *
+ * On Windows, https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/putenv-wputenv?view=msvc-170
+ * says
+ *
+ * The _putenv and _getenv families of functions are not thread-safe.
+ * _getenv could return a string pointer while _putenv is modifying the string,
+ * causing random failures. Make sure that calls to these functions are synchronized.
+ *
+ * Finally, we use a function-local static rather than a global so we don't have to think about /
+ * worry about whether global initialization was done correctly on the mutex object.
+ * This "worry" might be unfounded.
+ */
+std::mutex &MutexForEnvInvocations() {
+ static std::mutex the_mutex;
+ return the_mutex;
+}
+} // namespace
+
 std::optional<std::string> GetEnv(const std::string& envname) {
 #if defined(__unix__)
+ // Protect against concurrent XXXEnv() calls. See comment in MutexForEnvInvocations()
+ std::unique_lock guard(MutexForEnvInvocations());
  const char* ret = getenv(envname.c_str());
  if (ret != nullptr) {
  return std::string(ret);
  }
  return {};
 #elif defined(_WIN32)
+ // Protect against concurrent XXXEnv() calls. See comment in MutexForEnvInvocations()
+ std::unique_lock guard(MutexForEnvInvocations());
  static char ret[1024];
  size_t len;
  const errno_t err = getenv_s(&len, ret, sizeof(ret), envname.c_str());
- if (err == 0) {
- return std::string(ret);
+ // Return an unset optional if there's an error, or if the key is not found.
+ // len == 0 means "key not found" on Windows.
+ if (err != 0 || len == 0) {
+ return {};
  }
- return {};
+ return std::string(ret);
+#else
+#error "Unsupported configuration"
+#endif
+}
+
+void SetEnv(const std::string& envname, const std::string& value) {
+#if defined(__unix__)
+ // Protect against concurrent XXXEnv() calls. See comment in MutexForEnvInvocations()
+ std::unique_lock guard(MutexForEnvInvocations());
+
+ auto res = setenv(envname.c_str(), value.c_str(), 1);
+ if (res != 0) {
+ auto message = fmt::format("setenv failed, error={}", strerror(errno));
+ throw std::runtime_error(DEEPHAVEN_LOCATION_STR(message));
+ }
+#elif defined(_WIN32)
+ // Protect against concurrent XXXEnv() calls. See comment in MutexForEnvInvocations()
+ std::unique_lock guard(MutexForEnvInvocations());
+
+ auto res = _putenv_s(envname.c_str(), value.c_str());
+ if (res != 0) {
+ int lasterr = WSAGetLastError();
+ auto message = fmt::format("_putenv_s failed: error code {}", lasterr);
+ throw std::runtime_error(DEEPHAVEN_LOCATION_STR(message));
+ }
+#else
+#error "Unsupported configuration"
+#endif
+}
+
+void UnsetEnv(const std::string& envname) {
+#if defined(__unix__)
+ // Protect against concurrent XXXEnv() calls. See comment in MutexForEnvInvocations()
+ std::unique_lock guard(MutexForEnvInvocations());
+
+ auto res = unsetenv(envname.c_str());
+ if (res != 0) {
+ auto message = fmt::format("unsetenv failed, error={}", strerror(errno));
+ throw std::runtime_error(DEEPHAVEN_LOCATION_STR(message));
+ }
+#elif defined(_WIN32)
+ SetEnv(envname, "");
 #else
 #error "Unsupported configuration"
 #endif

cpp-client/deephaven/tests/src/utility_test.cc

@@ -11,6 +11,8 @@ using deephaven::dhcore::utility::GetEnv;
 using deephaven::dhcore::utility::GetTidAsString;
 using deephaven::dhcore::utility::GetHostname;
 using deephaven::dhcore::utility::ObjectId;
+using deephaven::dhcore::utility::SetEnv;
+using deephaven::dhcore::utility::UnsetEnv;
 
 namespace deephaven::client::tests {
 TEST_CASE("Base64encode", "[utility]") {
@@ -66,12 +68,43 @@ TEST_CASE("GetHostname", "[utility]") {
 // 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 visually spot-check
-// that ireturns the right value.
+// that it returns the right value.
 TEST_CASE("GetEnv", "[utility]") {
- auto path = GetEnv("PATH");
- // Very suspect if neither Windows nor Linux has a PATH set in their
- // environment.
- REQUIRE(path.has_value());
- fmt::println("PATH is: {}", *path);
+#if defined(__unix__)
+ const char *expected_key = "PATH";
+#elif defined(_WIN32)
+ const char *expected_key = "OS";
+#else
+#error "Unsupported configuration"
+#endif
+
+ auto value = GetEnv(expected_key);
+ REQUIRE(value.has_value());
+ fmt::println("{} is: {}", expected_key, *value);
+}
+
+// Confirm that SetEnv leaves something that GetEnv can find.
+TEST_CASE("SetEnv", "[utility]") {
+ std::string unlikely_key = "Deephaven__serious_realtime_data_tools";
+ std::string unlikely_value = "query_engine_APIs_and_user_interfaces";
+ {
+ auto value = GetEnv(unlikely_key);
+ if (value.has_value()) {
+ fmt::println(std::cerr, "unexpected value is {}", *value);
+ }
+ REQUIRE(!value.has_value());
+ }
+
+ SetEnv(unlikely_key, unlikely_value);
+ {
+ auto value = GetEnv(unlikely_key);
+ REQUIRE(unlikely_value == value);
+ }
+
+ UnsetEnv(unlikely_key);
+ {
+ auto value = GetEnv(unlikely_key);
+ REQUIRE(!value.has_value());
+ }
 }
 } // namespace deephaven::client::tests