[vm] Explicitly exclude interrupt related state in PRODUCT

This is follow up for the previous change somewhat incorrectly used
FLAG_profiler in ~OSThread to determine whether we need to delete
interrupter related state. FLAG_profiler is mutable in non-PRODUCT
builds so this could create memory leak.

TEST=testing PRODUCT and non-PRODUCT builds manually

Change-Id: Icf5ca6b83daab91daa125755261700ba2dbb9533
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/328422
Reviewed-by: Martin Kustermann <[email protected]>
Commit-Queue: Slava Egorov <[email protected]>

Author: mraleph

runtime/vm/dart_api_impl.cc

@@ -1593,19 +1593,23 @@ DART_EXPORT void Dart_StopProfiling() {
 }
 
 DART_EXPORT void Dart_ThreadDisableProfiling() {
+#if !defined(PRODUCT)
   OSThread* os_thread = OSThread::Current();
   if (os_thread == nullptr) {
     return;
   }
   os_thread->DisableThreadInterrupts();
+#endif  // !defined(PRODUCT)
 }
 
 DART_EXPORT void Dart_ThreadEnableProfiling() {
+#if !defined(PRODUCT)
   OSThread* os_thread = OSThread::Current();
   if (os_thread == nullptr) {
     return;
   }
   os_thread->EnableThreadInterrupts();
+#endif  // !defined(PRODUCT)
 }
 
 DART_EXPORT void Dart_AddSymbols(const char* dso_name,

runtime/vm/os_thread.cc

@@ -43,7 +43,6 @@ OSThread::OSThread()
 #endif
       name_(OSThread::GetCurrentThreadName()),
       timeline_block_lock_(),
-      thread_interrupt_disabled_(1),  // Thread interrupts disabled by default.
       log_(new class Log()) {
   // Try to get accurate stack bounds from pthreads, etc.
   if (!GetCurrentStackBounds(&stack_limit_, &stack_base_)) {
@@ -97,11 +96,13 @@ OSThread::~OSThread() {
 #endif
   timeline_block_ = nullptr;
   free(name_);
-  if (FLAG_profiler && prepared_for_interrupts_) {
+#if !defined(PRODUCT)
+  if (prepared_for_interrupts_) {
     ThreadInterrupter::CleanupCurrentThreadState(thread_interrupter_state_);
     thread_interrupter_state_ = nullptr;
     prepared_for_interrupts_ = false;
   }
+#endif  // !defined(PRODUCT)
 }
 
 void OSThread::SetName(const char* name) {
@@ -132,6 +133,7 @@ uword OSThread::GetCurrentStackPointer() {
   return stack_allocated_local;
 }
 
+#if !defined(PRODUCT)
 void OSThread::DisableThreadInterrupts() {
   ASSERT(OSThread::Current() == this);
   thread_interrupt_disabled_.fetch_add(1u);
@@ -159,6 +161,7 @@ void OSThread::EnableThreadInterrupts() {
 bool OSThread::ThreadInterruptsEnabled() {
   return thread_interrupt_disabled_ == 0;
 }
+#endif  // !defined(PRODUCT)
 
 static void DeleteThread(void* thread) {
   MSAN_UNPOISON(&thread, sizeof(thread));

runtime/vm/os_thread.h

@@ -148,10 +148,12 @@ class OSThread : public BaseThread {
   static void SetCurrentSafestackPointer(uword ssp);
 #endif
 
+#if !defined(PRODUCT)
   // Used to temporarily disable or enable thread interrupts.
   void DisableThreadInterrupts();
   void EnableThreadInterrupts();
   bool ThreadInterruptsEnabled();
+#endif  // !defined(PRODUCT)
 
   // The currently executing thread, or nullptr if not yet initialized.
   static OSThread* TryCurrent() {
@@ -296,9 +298,13 @@ class OSThread : public BaseThread {
   // All |Thread|s are registered in the thread list.
   OSThread* thread_list_next_ = nullptr;
 
-  RelaxedAtomic<uintptr_t> thread_interrupt_disabled_;
+#if !defined(PRODUCT)
+  // Thread interrupts disabled by default.
+  RelaxedAtomic<uintptr_t> thread_interrupt_disabled_ = {1};
   bool prepared_for_interrupts_ = false;
   void* thread_interrupter_state_ = nullptr;
+#endif  // !defined(PRODUCT)
+
   Log* log_;
   uword stack_base_ = 0;
   uword stack_limit_ = 0;

runtime/vm/thread.cc

@@ -561,7 +561,7 @@ void Thread::ResumeThreadInternal(Thread* thread) {
   thread->set_os_thread(os_thread);
   os_thread->set_thread(thread);
   Thread::SetCurrent(thread);
-  os_thread->EnableThreadInterrupts();
+  NOT_IN_PRODUCT(os_thread->EnableThreadInterrupts());
 
 #if !defined(PRODUCT) || defined(FORCE_INCLUDE_SAMPLING_HEAP_PROFILER)
   thread->heap_sampler().Initialize();
@@ -577,7 +577,7 @@ void Thread::SuspendThreadInternal(Thread* thread, VMTag::VMTagId tag) {
 
   OSThread* os_thread = thread->os_thread();
   ASSERT(os_thread != nullptr);
-  os_thread->DisableThreadInterrupts();
+  NOT_IN_PRODUCT(os_thread->DisableThreadInterrupts());
   os_thread->set_thread(nullptr);
   OSThread::SetCurrent(os_thread);
   thread->set_os_thread(nullptr);
@@ -1389,6 +1389,7 @@ void Thread::ResetDartMutatorState(Isolate* isolate) {
   ONLY_IN_PRECOMPILED(dispatch_table_array_ = nullptr);
 }
 
+#if !defined(PRODUCT)
 DisableThreadInterruptsScope::DisableThreadInterruptsScope(Thread* thread)
     : StackResource(thread) {
   if (thread != nullptr) {
@@ -1405,6 +1406,7 @@ DisableThreadInterruptsScope::~DisableThreadInterruptsScope() {
     os_thread->EnableThreadInterrupts();
   }
 }
+#endif
 
 NoReloadScope::NoReloadScope(Thread* thread) : ThreadStackResource(thread) {
 #if !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)

runtime/vm/thread.h

@@ -1498,12 +1498,21 @@ class RuntimeCallDeoptScope : public StackResource {
 void WindowsThreadCleanUp();
 #endif
 
+#if !defined(PRODUCT)
 // Disable thread interrupts.
 class DisableThreadInterruptsScope : public StackResource {
  public:
   explicit DisableThreadInterruptsScope(Thread* thread);
   ~DisableThreadInterruptsScope();
 };
+#else
+class DisableThreadInterruptsScope : public StackResource {
+ public:
+  explicit DisableThreadInterruptsScope(Thread* thread)
+      : StackResource(thread) {}
+  ~DisableThreadInterruptsScope() {}
+};
+#endif  // !defined(PRODUCT)
 
 // Within a NoSafepointScope, the thread must not reach any safepoint. Used
 // around code that manipulates raw object pointers directly without handles.