In Rubinius we're seeing an occasional crash inside LLVM that always happens inside getenv(), which is used for example when creating a MCContext (inside lib/MC/MCContext.cpp, it checks getenv("AS_SECURE_LOG_FILE")).
The problem is that getenv() and friends aren't thread safe and Rubinius provides a multithreaded system. We can relatively easily get locking setup around the getenv() calls we do in Rubinius, but that's really complex to be able to do for LLVM. I also don't know what the guarantees are here that LLVM wants to provide regarding thread safety of code that happens to have a getenv() call inside it.
What would be the best approach to solving this? Would it be necessary to put locking around this inside our usage of LLVM? Should LLVM provide a locking mechanism for this or should there be a way to make the getenv() calls optional in the places there are used (like here in MCContext).
Regarding the thread safety, this is what the open group says about getenv():
"The getenv() function need not be reentrant. A function that is not required to be reentrant is not required to be thread-safe."
That sounds like a missed multi-threading issue with LLVM. I can’t imagine why the user should be forced to serialize creation of MCContext objects. I would suggest filing a bug for this. A simple lock probably wouldn’t be too detrimental to performance here, since MCContext objects shouldn’t be created too often.
Right. glibc’s amusing stance is that you setenv/putenv are not thread safe, but getenv is. I assume Ruby exposes setenv and therefore simply not calling setenv isn’t an option.
Would it solve your problems if all getenv() calls happened at cl::ParseCommandLineOptions() time?
first place? It goes against LLVM's library-based design. So I don't think
introducing locking around this is the "right" solution. Can we just make
this value an argument to the constructor?
-- Sean Silva
This would require threading the value through some LLVMTargetMachine functions, at the least. Do we still lock and check the env. var. if the given value is NULL? Or just do away with the default coming from the env. var?
MCContext should definitely not be calling getenv. Yes, it should just be an argument to the ctor, or perhaps a “setter” method on mccontext, used by clients who care.
Yeah, Ruby allows basically for arbitrary getenv / setenv calls, so they can happen at any time. But since is though the Ruby API's, we can lock around this to prevent issues like this. Not really possible with MCContext, unless locking with the same lock around that as well then.
I think ParseCommandLineOptions would be fine, since we don't use that and I guess it's a one time thing to run at startup then right?