webaudio: Destroy audio handlers with the graph lock held.

This allows DeferredTaskHandler::RemoveMarkedSummingJunction to
assume that the graph lock is held, rather than having to acquire
it (recursively, if it's already held).

Since Oilpan finalizers cannot safely touch other heap objects (as
those object may be swept in the same GC cycle), the AudioNode
and AudioParam objects now hold a direct reference to the
DeferredTaskHandler associated with the audio context, which is
needed to acquire the appropriate lock.

Bug: 856641
Change-Id: Ied445a684d60b8beccb080b98867fcf80f967d21
Reviewed-on: https://chromium-review.googlesource.com/1182296
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Raymond Toy <rtoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585069}

third_party/blink/renderer/modules/webaudio/audio_node.cc

@@ -593,7 +593,18 @@ unsigned AudioHandler::NumberOfOutputChannels() const {
 // ----------------------------------------------------------------
 
 AudioNode::AudioNode(BaseAudioContext& context)
-    : context_(context), handler_(nullptr) {}
+    : context_(context),
+      deferred_task_handler_(&context.GetDeferredTaskHandler()),
+      handler_(nullptr) {}
+
+AudioNode::~AudioNode() {
+  // The graph lock is required to destroy the handler. And we can't use
+  // |context_| to touch it, since that object may also be a dead heap object.
+  {
+    DeferredTaskHandler::GraphAutoLocker locker(*deferred_task_handler_);
+    handler_ = nullptr;
+  }
+}
 
 void AudioNode::Dispose() {
   DCHECK(IsMainThread());

third_party/blink/renderer/modules/webaudio/audio_node.h

@@ -47,6 +47,7 @@ class AudioNodeOptions;
 class AudioNodeInput;
 class AudioNodeOutput;
 class AudioParam;
+class DeferredTaskHandler;
 class ExceptionState;
 
 // An AudioNode is the basic building block for handling audio within an
@@ -314,6 +315,8 @@ class MODULES_EXPORT AudioNode : public EventTargetWithInlineData {
   USING_PRE_FINALIZER(AudioNode, Dispose);
 
  public:
+  ~AudioNode() override;
+
   void Trace(blink::Visitor*) override;
   AudioHandler& Handler() const;
 
@@ -367,7 +370,12 @@ class MODULES_EXPORT AudioNode : public EventTargetWithInlineData {
   bool DisconnectFromOutputIfConnected(unsigned output_index, AudioParam&);
 
   Member<BaseAudioContext> context_;
+
+  // Needed in the destructor, where |context_| is not guaranteed to be alive.
+  scoped_refptr<DeferredTaskHandler> deferred_task_handler_;
+
   scoped_refptr<AudioHandler> handler_;
+
   // Represents audio node graph with Oilpan references. N-th HeapHashSet
   // represents a set of AudioNode objects connected to this AudioNode's N-th
   // output.

third_party/blink/renderer/modules/webaudio/audio_param.cc

@@ -339,7 +339,8 @@ AudioParam::AudioParam(BaseAudioContext& context,
                                          rate_mode,
                                          min_value,
                                          max_value)),
-      context_(context) {}
+      context_(context),
+      deferred_task_handler_(&context.GetDeferredTaskHandler()) {}
 
 AudioParam* AudioParam::Create(BaseAudioContext& context,
                                AudioParamType param_type,
@@ -364,6 +365,15 @@ AudioParam* AudioParam::Create(BaseAudioContext& context,
                         min_value, max_value);
 }
 
+AudioParam::~AudioParam() {
+  // The graph lock is required to destroy the handler. And we can't use
+  // |context_| to touch it, since that object may also be a dead heap object.
+  {
+    DeferredTaskHandler::GraphAutoLocker locker(*deferred_task_handler_);
+    handler_ = nullptr;
+  }
+}
+
 void AudioParam::Trace(blink::Visitor* visitor) {
   visitor->Trace(context_);
   ScriptWrappable::Trace(visitor);

third_party/blink/renderer/modules/webaudio/audio_param.h

@@ -270,6 +270,8 @@ class AudioParam final : public ScriptWrappable {
       float min_value = -std::numeric_limits<float>::max(),
       float max_value = std::numeric_limits<float>::max());
 
+  ~AudioParam() override;
+
   void Trace(blink::Visitor*) override;
   // |handler| always returns a valid object.
   AudioParamHandler& Handler() const { return *handler_; }
@@ -325,6 +327,8 @@ class AudioParam final : public ScriptWrappable {
   scoped_refptr<AudioParamHandler> handler_;
   Member<BaseAudioContext> context_;
 
+  // Needed in the destructor, where |context_| is not guaranteed to be alive.
+  scoped_refptr<DeferredTaskHandler> deferred_task_handler_;
 };
 
 }  // namespace blink

third_party/blink/renderer/modules/webaudio/audio_summing_junction.cc

@@ -33,6 +33,7 @@ AudioSummingJunction::AudioSummingJunction(DeferredTaskHandler& handler)
     : deferred_task_handler_(&handler), rendering_state_need_updating_(false) {}
 
 AudioSummingJunction::~AudioSummingJunction() {
+  DCHECK(GetDeferredTaskHandler().IsGraphOwner());
   GetDeferredTaskHandler().RemoveMarkedSummingJunction(this);
 }
 

third_party/blink/renderer/modules/webaudio/base_audio_context.cc

@@ -109,6 +109,13 @@ BaseAudioContext::BaseAudioContext(Document* document,
       output_position_() {}
 
 BaseAudioContext::~BaseAudioContext() {
+  {
+    // We may need to destroy summing junctions, which must happen while this
+    // object is still valid and with the graph lock held.
+    GraphAutoLocker locker(this);
+    destination_handler_ = nullptr;
+  }
+
   GetDeferredTaskHandler().ContextWillBeDestroyed();
   DCHECK(!active_source_nodes_.size());
   DCHECK(!is_resolving_resume_promises_);

third_party/blink/renderer/modules/webaudio/deferred_task_handler.cc

@@ -101,7 +101,7 @@ void DeferredTaskHandler::MarkSummingJunctionDirty(
 void DeferredTaskHandler::RemoveMarkedSummingJunction(
     AudioSummingJunction* summing_junction) {
   DCHECK(IsMainThread());
-  GraphAutoLocker locker(*this);
+  DCHECK(IsGraphOwner());
   dirty_summing_junctions_.erase(summing_junction);
 }