From 188c5f55562fae85dbff3d00017c850c2e2c944f Mon Sep 17 00:00:00 2001 From: "augusto.yjh" Date: Mon, 29 Jul 2024 08:25:49 -0700 Subject: [PATCH] fix static variable client access vialotion in DamonConfigLoader (#965) Summary: As discussed in pytorch issue [#129626](https://github.com/pytorch/pytorch/issues/129626) , `updateThread` of `Config` will keep on accessing `client` after main thread quits. But when main thread quits, `client` will be destructed. Functions in `updateThread` may use a dangling pointer of `client`, which will cause invalid memory access, and finally, `Segment fault` occurs, a core file will be generated, which doesn't behave as expected. Pull Request resolved: https://github.com/pytorch/kineto/pull/965 Reviewed By: xuzhao9 Differential Revision: D60291572 Pulled By: aaronenyeshi fbshipit-source-id: d97291aa825deeb672eb8e2d8385633c2838396d --- libkineto/src/DaemonConfigLoader.cpp | 9 +++++---- libkineto/src/DaemonConfigLoader.h | 7 +++++++ 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/libkineto/src/DaemonConfigLoader.cpp b/libkineto/src/DaemonConfigLoader.cpp index 6d73073df..726d561ee 100644 --- a/libkineto/src/DaemonConfigLoader.cpp +++ b/libkineto/src/DaemonConfigLoader.cpp @@ -13,14 +13,15 @@ #include "Logger.h" #include "ConfigLoader.h" #include "DaemonConfigLoader.h" -#include "IpcFabricConfigClient.h" namespace KINETO_NAMESPACE { // TODO : implications of this singleton being thread safe on forks? -IpcFabricConfigClient* getConfigClient() { - static auto client = std::make_unique(); - return client.get(); +IpcFabricConfigClient* DaemonConfigLoader::getConfigClient() { + if (!configClient){ + configClient = std::make_unique(); + } + return configClient.get(); } std::string DaemonConfigLoader::readBaseConfig() { diff --git a/libkineto/src/DaemonConfigLoader.h b/libkineto/src/DaemonConfigLoader.h index 1d920e11a..e30833007 100644 --- a/libkineto/src/DaemonConfigLoader.h +++ b/libkineto/src/DaemonConfigLoader.h @@ -14,6 +14,9 @@ #if !USE_GOOGLE_LOG #include #endif // !USE_GOOGLE_LOG +#ifdef __linux__ +#include "IpcFabricConfigClient.h" +#endif // __linux__ namespace KINETO_NAMESPACE { @@ -53,7 +56,11 @@ class DaemonConfigLoader : public IDaemonConfigLoader { void setCommunicationFabric(bool enabled) override; + IpcFabricConfigClient* getConfigClient(); + static void registerFactory(); +private: + std::unique_ptr configClient; }; #endif // __linux__