Don't delete lsan thread-local data until it's no longer required

[fjricci]

Summary

The routines for thread destruction in the thread registry require
the lsan thread index, which is stored in pthread tls on OS X.
This means that we need to make sure that the lsan tls isn't destroyed
until after the thread registry tls. This change ensures that we
don't delete the lsan tls until we've finished destroying the thread
in the registry, ensuring that the destructor for the lsan tls runs
after the destructor for the thread registry tls.

This patch also adds a check to ensure that the thread ID is valid before
returning it in GetThreadID(), to ensure that the above behavior
is working correctly.

[fjricci]

Fix up some style issues

[alekseyshl]

lib/lsan/lsan_common_mac.cc

39

Just curious, is !ptr check necessary?

[fjricci]

lib/lsan/lsan_common_mac.cc

39

Good catch, according to the man page, looks like destructors are only called if the value of the key is non-NULL. I'll remove that check before merging.

lib/lsan/lsan_common_mac.cc

typedef struct {
   int disable_counter;
   u32 current_thread_id;
   AllocatorCache cache;
 } thread_local_data_t;
 
 static pthread_key_t key;
 static pthread_once_t key_once = PTHREAD_ONCE_INIT;
 
-static void make_tls_key() { CHECK_EQ(pthread_key_create(&key, NULL), 0); }
+// The main thread destructor requires the current thread id,
+// so we can't destroy it until it's been used and reset to invalid tid
+void keep_tid_alive(void *ptr) {
+  if (!ptr) {
+    return;
+  }
+
+  thread_local_data_t *data = (thread_local_data_t *)ptr;
+  if (data->current_thread_id != kInvalidTid) {
+    pthread_setspecific(key, data);
+  }
+}
+
+static void make_tls_key() {
+  CHECK_EQ(pthread_key_create(&key, keep_tid_alive), 0);
+}
 
 static thread_local_data_t *get_tls_val(bool alloc) {
   pthread_once(&key_once, make_tls_key);
 
   thread_local_data_t *ptr = (thread_local_data_t *)pthread_getspecific(key);
   if (ptr == NULL && alloc) {
     ptr = (thread_local_data_t *)InternalAlloc(sizeof(*ptr));
     ptr->disable_counter = 0;

 void EnableInThisThread() {
   int *disable_counter = &get_tls_val(true)->disable_counter;
   if (*disable_counter == 0) {
     DisableCounterUnderflow();
   }
   --*disable_counter;
 }
 
-u32 GetCurrentThread() { return get_tls_val(true)->current_thread_id; }
+u32 GetCurrentThread() {
+  thread_local_data_t *data = get_tls_val(false);
+  CHECK(data);
+  return data->current_thread_id;
+}
 
-void SetCurrentThread(u32 tid) { get_tls_val(true)->current_thread_id = tid; }
+void SetCurrentThread(u32 tid) {
+  get_tls_val(true)->current_thread_id = tid;
+}
 
 AllocatorCache *GetAllocatorCache() { return &get_tls_val(true)->cache; }
 
 // Required on Linux for initialization of TLS behavior, but should not be
 // required on Darwin.
 void InitializePlatformSpecificModules() {}
 
 // Scans global variables for heap pointers.

lib/lsan/lsan_thread.cc

void ThreadStart(u32 tid, uptr os_id) {
   args.tls_end = args.tls_begin + tls_size;
   GetAllocatorCacheRange(&args.cache_begin, &args.cache_end);
   args.dtls = DTLS_Get();
   thread_registry->StartThread(tid, os_id, /*workerthread*/ false, &args);
 }
 
 void ThreadFinish() {
   thread_registry->FinishThread(GetCurrentThread());
+  SetCurrentThread(kInvalidTid);
 }
 
 ThreadContext *CurrentThreadContext() {
   if (!thread_registry) return nullptr;
   if (GetCurrentThread() == kInvalidTid)
     return nullptr;
   // No lock needed when getting current thread.
   return (ThreadContext *)thread_registry->GetThreadLocked(GetCurrentThread());