Skip to content

Commit

Permalink
Don't call getenv in side threads (#984)
Browse files Browse the repository at this point in the history
Summary:
Calling `getenv` on side threads is dangerous as it can potentially segfault if the main thread is in the middle of setting environment variables: pytorch/pytorch#134596

This PR only calls `getenv` only once during the first call of  `isDaemonEnvVarSet()`, which is called from `init`.

Pull Request resolved: #984

Reviewed By: sraikund16

Differential Revision: D62152169

Pulled By: malfet

fbshipit-source-id: 28dff07cb9775b004580749805b6437dba978eeb

Co-authored-by: Nikita Shulga <[email protected]>
  • Loading branch information
2 people authored and facebook-github-bot committed Oct 2, 2024
1 parent b5c85da commit 86f1deb
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 2 deletions.
4 changes: 4 additions & 0 deletions libkineto/include/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -502,4 +502,8 @@ class Config : public AbstractConfig {

constexpr char kUseDaemonEnvVar[] = "KINETO_USE_DAEMON";

#if __linux__
bool isDaemonEnvVarSet();
#endif

} // namespace libkineto
13 changes: 12 additions & 1 deletion libkineto/src/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,10 +243,21 @@ Config::Config()
factories->addFeatureConfigs(*this);
}
#if __linux__
enableIpcFabric_ = (getenv(kUseDaemonEnvVar) != nullptr);
enableIpcFabric_ = libkineto::isDaemonEnvVarSet();
#endif
}

#if __linux__
bool isDaemonEnvVarSet() {
static bool rc = [] {
void *ptr = getenv(kUseDaemonEnvVar);
return ptr != nullptr;
}();
return rc;
}
#endif


std::shared_ptr<void> Config::getStaticObjectsLifetimeHandle() {
return configFactories();
}
Expand Down
2 changes: 1 addition & 1 deletion libkineto/src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void libkineto_init(bool cpuOnly, bool logOnError) {

// Factory to connect to open source daemon if present
#if __linux__
if (getenv(kUseDaemonEnvVar) != nullptr) {
if (libkineto::isDaemonEnvVarSet()) {
LOG(INFO) << "Registering daemon config loader, cpuOnly = "
<< cpuOnly;
DaemonConfigLoader::registerFactory();
Expand Down

0 comments on commit 86f1deb

Please sign in to comment.