GetEnv is hazardous when used in multiple threads

[giriel]

Hi all,

While using this library to read environment variables from multiple threads at the same time we noticed crashes and jumbled results.

Looking at the implementation, the static stackstring value is to blame.
I understand that this is done to keep the non-owning pointer semantics similar to the std version.

But the std version is a bit more thread aware since c++11: https://en.cppreference.com/w/cpp/utility/program/getenv

Should a similar guarantee not be given?
Or at the very least a big warning be added that you should synchronise access to this function.
A last option would be to foresee a version that returns the environment value as an owning buffer.

Kind regards,
Robin

[Flamefire]

Thanks for opening the issue and you are right: In addition to the value returned possibly being modified by putenv etc. different threads calling getenv might corrupt the returned data. This is at least documented:

/// This function is not thread safe or reenterable as defined by the standard library

Give the library requires C++11 now it can be improved to be at least thread-safe.

[artyom-beilis]

But making getenv thread safe on windows only can lead to unexpected behavior. On posix systems or normal ansi or wide versions of getenv aren't thread safe.

What good would it do making it thread safe on single platform creating unexpected behavior

[Flamefire]

It already is thread-safe on non-Windows which was the reason of the original report as far as I understood:

This function is thread-safe (calling it from multiple threads does not introduce a data race) as long as no other function modifies the host environment. In particular, the POSIX functions setenv(), unsetenv(), and putenv() would introduce a data race if called without synchronization. (since C++11)

So this to me makes the behaviour consistent

[artyom-beilis]

I understand what you mean. getenv should be safe as long as there is no race condition.

Yes, I got you. You are correct