Do Not Cache PID/TID in Logging (#994)

Summary:
Pull Request resolved: #994

S451588 was caused by the LOG macro caching values that would then be copied to other processes via forking. In general, we should probably use fork handlers to clear out said variables, but from a hygiene point of view we should also not be changing control flow based on logging. For this reason, the pid/tid retrieval in logging should get the cached variable if it exists, but never do the caching itself.

Reviewed By: aaronenyeshi

Differential Revision: D63668265

fbshipit-source-id: 6817c743248056464213be28f562d07752ec3283

Author: sraikund16

libkineto/include/ThreadUtil.h

@@ -15,12 +15,12 @@
 
 namespace libkineto {
 
-int32_t systemThreadId();
+int32_t systemThreadId(bool cache=true);
 int32_t threadId();
 bool setThreadName(const std::string& name);
 std::string getThreadName();
 
-int32_t processId();
+int32_t processId(bool cache=true);
 std::string processName(int32_t pid);
 
 // Return a list of pids and process names for the current process

libkineto/src/Logger.cpp

@@ -39,7 +39,7 @@ Logger::Logger(int severity, int line, const char* filePath, int errnum)
       std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
   const char* file = strrchr(filePath, '/');
   buf_ << fmt::format("{:%Y-%m-%d %H:%M:%S}", fmt::localtime(tt)) << " "
-       << processId() << ":" << systemThreadId() << " "
+       << processId(false) << ":" << systemThreadId(false) << " "
        << (file ? file + 1 : filePath) << ":" << line << "] ";
 }
 

libkineto/src/ThreadUtil.cpp

@@ -39,28 +39,38 @@ thread_local int32_t _tid = 0;
 thread_local int32_t _sysTid = 0;
 }
 
-int32_t processId() {
+int32_t processId(bool cache) {
+  int32_t pid = 0;
   if (!_pid) {
 #ifndef _WIN32
-    _pid = (int32_t)getpid();
+    pid = (int32_t)getpid();
 #else
-    _pid = (int32_t)GetCurrentProcessId();
+    pid = (int32_t)GetCurrentProcessId();
 #endif
+    if (cache) {
+      _pid = pid;
+    }
+    return pid;
   }
   return _pid;
 }
 
-int32_t systemThreadId() {
+int32_t systemThreadId(bool cache) {
+  int32_t sysTid = 0;
   if (!_sysTid) {
 #ifdef __APPLE__
-    _sysTid = (int32_t)syscall(SYS_thread_selfid);
+    sysTid = (int32_t)syscall(SYS_thread_selfid);
 #elif defined _WIN32
-    _sysTid = (int32_t)GetCurrentThreadId();
+    sysTid = (int32_t)GetCurrentThreadId();
 #elif defined __FreeBSD__
-    syscall(SYS_thr_self, &_sysTid);
+    syscall(SYS_thr_self, &sysTid);
 #else
-    _sysTid = (int32_t)syscall(SYS_gettid);
+    sysTid = (int32_t)syscall(SYS_gettid);
 #endif
+    if (cache) {
+      _sysTid = sysTid;
+    }
+    return sysTid;
   }
   return _sysTid;
 }