Avoid unsafe heap access from audio thread.

third_party/WebKit/Source/modules/webaudio/BaseAudioContext.cpp

 /*
  * Copyright (C) 2010, Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 77 matching lines @@
 
 // FIXME(dominicc): Devolve these constructors to AudioContext
 // and OfflineAudioContext respectively.
 
 // Constructor for rendering to the audio hardware.
 BaseAudioContext::BaseAudioContext(Document* document)
     : SuspendableObject(document),
       destination_node_(nullptr),
       is_cleared_(false),
       is_resolving_resume_promises_(false),
+      has_posted_cleanup_task_(false),
       user_gesture_required_(false),
       connection_count_(0),
       deferred_task_handler_(DeferredTaskHandler::Create()),
       context_state_(kSuspended),
       closed_context_sample_rate_(-1),
       periodic_wave_sine_(nullptr),
       periodic_wave_square_(nullptr),
       periodic_wave_sawtooth_(nullptr),
       periodic_wave_triangle_(nullptr),
       output_position_() {
@@ skipping 644 matching lines @@
 
   NOTREACHED();
   return false;
 }
 
 bool BaseAudioContext::ReleaseFinishedSourceNodes() {
   DCHECK(IsGraphOwner());
   DCHECK(IsAudioThread());
   bool did_remove = false;
   for (AudioHandler* handler : finished_source_handlers_) {
+    // A main thread GC must not be running while the audio
+    // thread iterates over the |active_source_nodes_| heap object.
+    ThreadState::GCLockScope gc_lock(ThreadState::MainThreadState());

[haraken, 2017/06/08 01:07:55]

Add a TODO to remove this somehow. We should refactor code so that the audio
thread doesn't need to access data structures on Oilpan's heap.

GCLockScope is not nice because the audio thread needs to stop while the main
thread is running a GC. This breaks the realtime-ness of the audio thread.

Also it violates Blink's messaging passing model. In Blink a thread should not
access data structures held by a different thread.

+
     for (AudioNode* node : active_source_nodes_) {
       if (finished_source_nodes_.Contains(node))
         continue;
       if (handler == &node->Handler()) {
         handler->BreakConnection();
         finished_source_nodes_.insert(node);
         did_remove = true;
         break;
       }
     }
@@ skipping 12 matching lines @@
 
 void BaseAudioContext::ReleaseActiveSourceNodes() {
   DCHECK(IsMainThread());
   for (auto& source_node : active_source_nodes_)
     source_node->Handler().BreakConnection();
 
   active_source_nodes_.clear();
 }
 
 void BaseAudioContext::HandleStoppableSourceNodes() {
+  DCHECK(IsAudioThread());
   DCHECK(IsGraphOwner());
 
-  // Find AudioBufferSourceNodes to see if we can stop playing them.
-  for (AudioNode* node : active_source_nodes_) {
-    // If the AudioNode has been marked as finished and released by
-    // the audio thread, but not yet removed by the main thread
-    // (see releaseActiveSourceNodes() above), |node| must not be
-    // touched as its handler may have been released already.
-    if (finished_source_nodes_.Contains(node))
-      continue;
-    if (node->Handler().GetNodeType() ==
-        AudioHandler::kNodeTypeAudioBufferSource) {
-      AudioBufferSourceNode* source_node =
-          static_cast<AudioBufferSourceNode*>(node);
-      source_node->GetAudioBufferSourceHandler().HandleStoppableSourceNode();
-    }
-  }
+  ScheduleMainThreadCleanup();
 }
 
 void BaseAudioContext::HandlePreRenderTasks(
     const AudioIOPosition& output_position) {
   DCHECK(IsAudioThread());
 
   // At the beginning of every render quantum, try to update the internal
   // rendering graph state (from main thread changes).  It's OK if the tryLock()
   // fails, we'll just take slightly longer to pick up the changes.
   if (TryLock()) {
@@ skipping 33 matching lines @@
 
     GetDeferredTaskHandler().HandleDeferredTasks();
     GetDeferredTaskHandler().RequestToDeleteHandlersOnMainThread();
 
     unlock();
   }
 
   RemoveFinishedSourceNodes(did_remove);
 }
 
-void BaseAudioContext::ResolvePromisesForResumeOnMainThread() {
+void BaseAudioContext::PerformCleanupOnMainThread() {
   DCHECK(IsMainThread());
   AutoLocker locker(this);
 
-  for (auto& resolver : resume_resolvers_) {
-    if (context_state_ == kClosed) {
-      resolver->Reject(DOMException::Create(
-          kInvalidStateError, "Cannot resume a context that has been closed"));
-    } else {
-      SetContextState(kRunning);
-      resolver->Resolve();
+  if (is_resolving_resume_promises_) {
+    for (auto& resolver : resume_resolvers_) {
+      if (context_state_ == kClosed) {
+        resolver->Reject(DOMException::Create(
+            kInvalidStateError,
+            "Cannot resume a context that has been closed"));
+      } else {
+        SetContextState(kRunning);
+        resolver->Resolve();
+      }
+    }
+    resume_resolvers_.clear();
+    is_resolving_resume_promises_ = false;
+  }
+
+  // Find AudioBufferSourceNodes to see if we can stop playing them.
+  for (AudioNode* node : active_source_nodes_) {
+    // If the AudioNode has been marked as finished and released by
+    // the audio thread, but not yet removed by the main thread
+    // (see releaseActiveSourceNodes() above), |node| must not be
+    // touched as its handler may have been released already.
+    if (finished_source_nodes_.Contains(node))
+      continue;
+    if (node->Handler().GetNodeType() ==
+        AudioHandler::kNodeTypeAudioBufferSource) {
+      AudioBufferSourceNode* source_node =
+          static_cast<AudioBufferSourceNode*>(node);
+      source_node->GetAudioBufferSourceHandler().HandleStoppableSourceNode();
     }
   }
+  has_posted_cleanup_task_ = false;
+}
 
-  resume_resolvers_.clear();
-  is_resolving_resume_promises_ = false;
+void BaseAudioContext::ScheduleMainThreadCleanup() {
+  if (has_posted_cleanup_task_)
+    return;
+  Platform::Current()->MainThread()->GetWebTaskRunner()->PostTask(
+      BLINK_FROM_HERE,
+      CrossThreadBind(&BaseAudioContext::PerformCleanupOnMainThread,
+                      WrapCrossThreadPersistent(this)));
+  has_posted_cleanup_task_ = true;
 }
 
 void BaseAudioContext::ResolvePromisesForResume() {
   // This runs inside the BaseAudioContext's lock when handling pre-render
   // tasks.
   DCHECK(IsAudioThread());
   DCHECK(IsGraphOwner());
 
   // Resolve any pending promises created by resume(). Only do this if we
   // haven't already started resolving these promises. This gets called very
   // often and it takes some time to resolve the promises in the main thread.
   if (!is_resolving_resume_promises_ && resume_resolvers_.size() > 0) {
     is_resolving_resume_promises_ = true;
-    Platform::Current()->MainThread()->GetWebTaskRunner()->PostTask(
-        BLINK_FROM_HERE,
-        CrossThreadBind(&BaseAudioContext::ResolvePromisesForResumeOnMainThread,
-                        WrapCrossThreadPersistent(this)));
+    ScheduleMainThreadCleanup();
   }
 }
 
 void BaseAudioContext::RejectPendingDecodeAudioDataResolvers() {
   // Now reject any pending decodeAudioData resolvers
   for (auto& resolver : decode_audio_resolvers_)
     resolver->Reject(DOMException::Create(kInvalidStateError,
                                           "Audio context is going away"));
   decode_audio_resolvers_.clear();
 }
@@ skipping 115 matching lines @@
 }
 
 SecurityOrigin* BaseAudioContext::GetSecurityOrigin() const {
   if (GetExecutionContext())
     return GetExecutionContext()->GetSecurityOrigin();
 
   return nullptr;
 }
 
 }  // namespace blink