Hold global GC heap lock while making audio thread access.

For auxillary threads that rarely need to gain access to another
thread's GC heap, we have to ensure that this happens while the
heap-owning thread isn't concurrently GCing the heap. Otherwise there
is the possibility that heap objects might be relocated or mutated
while the auxillary thread tries to access.

A CrossThreadPersistent<T> (CTP) ensures reference liveness across
threads, but isn't sufficient to handle the wanted exclusive access
after a non-attached thread has deref'ed the persistent. So, for
this to happen, keep the global CTP lock while accessing a heap
object during offline audio rendering -- it specifically accessing
heap objects while a GC runs. As all GCs hold the lock on the global
CTP region while they run, this ensures exclusion.

It is clearly desirable to have all heap access be under the control
of the heap-owning thread, and threaded code should try hard to avoid
accessing another thread's heap objects. The CTP global lock is the
mechanism to use when that isn't practically feasible -- feel free to
add a "TODO(foo): avoid using" next to any instances that you end up
introducing.

As regards audio thread cross-thread usage, the code needs auditing to
check if there are other places where setting up this CTP lock is
required.

R=haraken,rtoy,hongchan
BUG=732511

Review-Url: https://codereview.chromium.org/2951903003
Cr-Commit-Position: refs/heads/master@{#481809}

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

@@ -173,7 +173,23 @@ void OfflineAudioDestinationHandler::StartOfflineRendering() {
 void OfflineAudioDestinationHandler::DoOfflineRendering() {
   DCHECK(!IsMainThread());
 
-  unsigned number_of_channels = render_target_->numberOfChannels();
+  unsigned number_of_channels;
+  Vector<float*> destinations;
+  {
+    // Main thread GCs cannot happen while we're reading out channel
+    // data. Detect that condition by trying to take the cross-thread
+    // persistent lock which is held while a GC runs. If the lock is
+    // already held, simply delay rendering until the next quantum.
+    CrossThreadPersistentRegion::LockScope gc_lock(
+        ProcessHeap::GetCrossThreadPersistentRegion(), true);
+    if (!gc_lock.HasLock())
+      return;
+
+    number_of_channels = render_target_->numberOfChannels();
+    destinations.ReserveInitialCapacity(number_of_channels);
+    for (unsigned i = 0; i < number_of_channels; ++i)
+      destinations.push_back(render_target_->getChannelData(i).View()->Data());
+  }
 
   // Reset the suspend flag.
   should_suspend_ = false;
@@ -198,9 +214,7 @@ void OfflineAudioDestinationHandler::DoOfflineRendering() {
     for (unsigned channel_index = 0; channel_index < number_of_channels;
          ++channel_index) {
       const float* source = render_bus_->Channel(channel_index)->Data();
-      float* destination =
-          render_target_->getChannelData(channel_index).View()->Data();
-      memcpy(destination + frames_processed_, source,
+      memcpy(destinations[channel_index] + frames_processed_, source,
              sizeof(float) * frames_available_to_copy);
     }
 

third_party/WebKit/Source/platform/heap/PersistentNode.h

@@ -202,14 +202,27 @@ class CrossThreadPersistentRegion final {
     STACK_ALLOCATED();
 
    public:
-    LockScope(CrossThreadPersistentRegion& persistent_region)
-        : persistent_region_(persistent_region) {
-      persistent_region_.lock();
+    LockScope(CrossThreadPersistentRegion& persistent_region,
+              bool try_lock = false)
+        : persistent_region_(persistent_region), locked_(true) {
+      if (try_lock)
+        locked_ = persistent_region_.TryLock();
+      else
+        persistent_region_.lock();
     }
-    ~LockScope() { persistent_region_.unlock(); }
+    ~LockScope() {
+      if (locked_)
+        persistent_region_.unlock();
+    }
+
+    // If the lock scope is set up with |try_lock| set to |true|, caller/user
+    // is responsible for checking whether the GC lock was taken via
+    // |HasLock()|.
+    bool HasLock() const { return locked_; }
 
    private:
     CrossThreadPersistentRegion& persistent_region_;
+    bool locked_;
   };
 
   void TracePersistentNodes(Visitor* visitor) {
@@ -237,6 +250,8 @@ class CrossThreadPersistentRegion final {
 
   void unlock() { mutex_.unlock(); }
 
+  bool TryLock() { return mutex_.TryLock(); }
+
   // We don't make CrossThreadPersistentRegion inherit from PersistentRegion
   // because we don't want to virtualize performance-sensitive methods
   // such as PersistentRegion::allocate/freePersistentNode.

third_party/WebKit/Source/platform/heap/ThreadState.cpp

@@ -1453,9 +1453,11 @@ void ThreadState::CollectGarbage(BlinkGC::StackState stack_state,
   GCForbiddenScope gc_forbidden_scope(this);
 
   {
-    // Access to the CrossThreadPersistentRegion has to be prevented while in
-    // the marking phase because otherwise other threads may allocate or free
-    // PersistentNodes and we can't handle that.
+    // Access to the CrossThreadPersistentRegion has to be prevented
+    // while in the marking phase because otherwise other threads may
+    // allocate or free PersistentNodes and we can't handle
+    // that. Grabbing this lock also prevents non-attached threads
+    // from accessing any GCed heap while a GC runs.
     CrossThreadPersistentRegion::LockScope persistent_lock(
         ProcessHeap::GetCrossThreadPersistentRegion());
     {