Merge pull request #191 from boostorg/get-env-thread

Make `getenv` thread-safe

Author: Flamefire

.github/workflows/ci_tests.yml

@@ -108,7 +108,10 @@ jobs:
     # Run test with both bash and powershell and watch for "Using std::cin" on bash but not on powershell
     - name: Test
       working-directory: build
-      run: ctest --output-on-failure -C ${{matrix.buildType}} --verbose
+      run: |
+        # The bash shell adds an incompatible PATH for MinGW: https://github.com/actions/runner-images/issues/11102
+        [[ "${{runner.os}}" != 'Windows' ]] || export PATH="/c/mingw64/bin:$PATH"
+        ctest --output-on-failure -C ${{matrix.buildType}} --verbose
     - name: Test on PowerShell
       working-directory: build
       shell: powershell

doc/changelog.dox

@@ -1,5 +1,5 @@
 //
-// Copyright (c) 2019-2023 Alexander Grund
+// Copyright (c) 2019-2024 Alexander Grund
 //
 // Distributed under the Boost Software License, Version 1.0.
 // https://www.boost.org/LICENSE_1_0.txt
@@ -8,6 +8,10 @@
 
 \section changelog Changelog
 
+\subsection changelog_11_3_1 Nowide 11.3.1 (Boost 1.88)
+- Fix redefinition of `_CRT_SECURE_NO_WARNINGS`
+- Make `getenv` thread-safe
+
 \subsection changelog_11_3_0 Nowide 11.3.0 (Boost 1.82)
 - Add `convert_string` overload accepting a string
 - Add `quoted` to output (quoted) paths (std::filesystem or boost::filesystem)

include/boost/nowide/cstdlib.hpp

@@ -21,7 +21,12 @@ namespace nowide {
     ///
     /// \brief UTF-8 aware getenv. Returns 0 if the variable is not set.
     ///
-    /// This function is not thread safe or reenterable as defined by the standard library
+    /// The string pointed to shall not be modified by the program.
+    /// This function is thread-safe as long as no other thread modifies the host environment.
+    /// However subsequent calls to this function might overwrite the string pointed to.
+    ///
+    /// Warning: The returned pointer might only be valid for as long as the calling thread is alive.
+    ///          So avoid passing it across thread boundaries.
     ///
     BOOST_NOWIDE_DECL char* getenv(const char* key);
 

src/cstdlib.cpp

@@ -48,11 +48,46 @@ namespace nowide {
 #include <vector>
 #include <windows.h>
 
+namespace {
+// thread_local was broken on MinGW for all 32bit compiler releases prior to 11.x, see
+// https://sourceforge.net/p/mingw-w64/bugs/527/
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83562
+// Using a non-trivial destructor causes program termination on thread exit.
+#if defined(__MINGW32__) && !defined(__MINGW64__) && !defined(__clang__) && (__GNUC__ < 11)
+class stackstring_for_thread
+{
+    union
+    {
+        boost::nowide::stackstring s_;
+    };
+
+public:
+    stackstring_for_thread() : s_(){};
+    // Empty destructor so the union member (using a non-trivial destructor) does not get destroyed.
+    // This will leak memory if any is allocated by the stackstring for each terminated thread
+    // but as most values fit into the stack buffer this is rare and still better than a crash.
+    ~stackstring_for_thread(){};
+    void convert(const wchar_t* begin, const wchar_t* end)
+    {
+        s_.convert(begin, end);
+    }
+
+    char* get()
+    {
+        return s_.get();
+    }
+};
+#else
+using stackstring_for_thread = boost::nowide::stackstring;
+#endif
+
+} // namespace
+
 namespace boost {
 namespace nowide {
     char* getenv(const char* key)
     {
-        static stackstring value;
+        thread_local stackstring_for_thread value;
 
         const wshort_stackstring name(key);
 
@@ -72,7 +107,7 @@ namespace nowide {
                 return 0;
             ptr = &tmp[0];
         }
-        value.convert(ptr);
+        value.convert(ptr, ptr + n);
         return value.get();
     }