[VM/Timeline] Fix race between Timeline::Cleanup() and OSThread's destructor

TEST=checked that the timeline still worked correctly in DevTools, used
ASAN and TSAN to check that this change doesn't introduce new problems

Issue: https://github.com/flutter/flutter/issues/136402
Change-Id: Ic63a067aed21fefa890c49c73a73f60979f1e809
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/330320
Commit-Queue: Derek Xu <derekx@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>

runtime/vm/os_thread.cc

@@ -85,10 +85,17 @@ OSThread::~OSThread() {
   delete log_;
   log_ = nullptr;
 #if defined(SUPPORT_TIMELINE)
-  if (Timeline::recorder() != nullptr) {
+  RecorderSynchronizationLockScope ls;
+  TimelineEventRecorder* recorder = Timeline::recorder();
+  if (recorder != nullptr && !ls.IsShuttingDown()) {
     // Acquire the recorder's lock so that |timeline_block_| cannot be given to
     // another thread until the call to |TimelineEventRecorder::FinishBlock| is
-    // complete.
+    // complete. This is guarded by a check ensuring that the recorder has not
+    // yet been deleted, and thus ensuring that the recorder's lock can be
+    // acquired. It is fine to skip the call to
+    // |TimelineEventRecorder::FinishBlock| if the recorder has already been
+    // deleted, because only the recorder cares about whether blocks have been
+    // marked as finished.
     MutexLocker recorder_lock_locker(&Timeline::recorder()->lock_);
     MutexLocker timeline_block_lock_locker(timeline_block_lock());
     Timeline::recorder()->FinishBlock(timeline_block_);

runtime/vm/timeline.h

@@ -173,6 +173,10 @@ class RecorderSynchronizationLock : public AllStatic {
     return (recorder_state_.load(std::memory_order_acquire) == kActive);
   }
 
+  static bool IsShuttingDown() {
+    return (recorder_state_.load(std::memory_order_acquire) == kShuttingDown);
+  }
+
   static void WaitForShutdown() {
     recorder_state_.store(kShuttingDown, std::memory_order_release);
     // Spin waiting for outstanding events to be completed.
@@ -207,6 +211,10 @@ class RecorderSynchronizationLockScope {
 
   bool IsActive() const { return RecorderSynchronizationLock::IsActive(); }
 
+  bool IsShuttingDown() const {
+    return RecorderSynchronizationLock::IsShuttingDown();
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(RecorderSynchronizationLockScope);
 };