Ensure that AudioNode deletion is synchronized with a stable state of the rendering...
authorcrogers@google.com <crogers@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2012 23:23:51 +0000 (23:23 +0000)
committercrogers@google.com <crogers@google.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 1 Nov 2012 23:23:51 +0000 (23:23 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100994

Reviewed by Kenneth Russell.

In some rare cases it has been observed that nodes are getting deleted in the main thread
during an audio rendering quantum where the dirty inputs and outputs have not yet been cleaned
via calls to handleDirtyAudioSummingJunctions() and handleDirtyAudioNodeOutputs().
This was possible because nodes marked for deletion with markForDeletion() could be picked
up in a subsequent call to deleteMarkedNodes() before the render quantum has finished and
handlePostRenderTasks() has had a chance to reconcile these marked nodes and clean the dirty state.
The solution is to manage the marked nodes in a separate vector which only gets copied to another
vector truly eligible for deletion which is synchronized in handlePostRenderTasks().

* Modules/webaudio/AudioContext.cpp:
(WebCore::AudioContext::markForDeletion):
(WebCore::AudioContext::scheduleNodeDeletion):
(WebCore::AudioContext::deleteMarkedNodes):
* Modules/webaudio/AudioContext.h:
(AudioContext):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@133239 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/Modules/webaudio/AudioContext.cpp
Source/WebCore/Modules/webaudio/AudioContext.h

index adffe32..c9faf5e 100644 (file)
@@ -1,3 +1,26 @@
+2012-11-01  Chris Rogers  <crogers@google.com>
+
+        Ensure that AudioNode deletion is synchronized with a stable state of the rendering graph
+        https://bugs.webkit.org/show_bug.cgi?id=100994
+
+        Reviewed by Kenneth Russell.
+
+        In some rare cases it has been observed that nodes are getting deleted in the main thread
+        during an audio rendering quantum where the dirty inputs and outputs have not yet been cleaned
+        via calls to handleDirtyAudioSummingJunctions() and handleDirtyAudioNodeOutputs().
+        This was possible because nodes marked for deletion with markForDeletion() could be picked
+        up in a subsequent call to deleteMarkedNodes() before the render quantum has finished and
+        handlePostRenderTasks() has had a chance to reconcile these marked nodes and clean the dirty state.
+        The solution is to manage the marked nodes in a separate vector which only gets copied to another
+        vector truly eligible for deletion which is synchronized in handlePostRenderTasks().
+
+        * Modules/webaudio/AudioContext.cpp:
+        (WebCore::AudioContext::markForDeletion):
+        (WebCore::AudioContext::scheduleNodeDeletion):
+        (WebCore::AudioContext::deleteMarkedNodes):
+        * Modules/webaudio/AudioContext.h:
+        (AudioContext):
+
 2012-11-01  Ryosuke Niwa  <rniwa@webkit.org>
 
         Build fix after r133224 as suggested by Enrica.
index fceddf3..9f65880 100644 (file)
@@ -764,7 +764,7 @@ void AudioContext::handleDeferredFinishDerefs()
 void AudioContext::markForDeletion(AudioNode* node)
 {
     ASSERT(isGraphOwner());
-    m_nodesToDelete.append(node);
+    m_nodesMarkedForDeletion.append(node);
 
     // This is probably the best time for us to remove the node from automatic pull list,
     // since all connections are gone and we hold the graph lock. Then when handlePostRenderTasks()
@@ -781,7 +781,11 @@ void AudioContext::scheduleNodeDeletion()
         return;
 
     // Make sure to call deleteMarkedNodes() on main thread.    
-    if (m_nodesToDelete.size() && !m_isDeletionScheduled) {
+    if (m_nodesMarkedForDeletion.size() && !m_isDeletionScheduled) {
+        for (unsigned i = 0; i < m_nodesMarkedForDeletion.size(); ++i)
+            m_nodesToDelete.append(m_nodesMarkedForDeletion[i]);
+        m_nodesMarkedForDeletion.clear();
+
         m_isDeletionScheduled = true;
 
         // Don't let ourself get deleted before the callback.
@@ -808,7 +812,6 @@ void AudioContext::deleteMarkedNodes()
 
     AutoLocker locker(this);
     
-    // Note: deleting an AudioNode can cause m_nodesToDelete to grow.
     while (size_t n = m_nodesToDelete.size()) {
         AudioNode* node = m_nodesToDelete[n - 1];
         m_nodesToDelete.removeLast();
index 267ee42..3098d8e 100644 (file)
@@ -286,6 +286,11 @@ private:
     Vector<AudioNode*> m_referencedNodes;
 
     // Accumulate nodes which need to be deleted here.
+    // This is copied to m_nodesToDelete at the end of a render cycle in handlePostRenderTasks(), where we're assured of a stable graph
+    // state which will have no references to any of the nodes in m_nodesToDelete once the context lock is released
+    // (when handlePostRenderTasks() has completed).
+    Vector<AudioNode*> m_nodesMarkedForDeletion;
+
     // They will be scheduled for deletion (on the main thread) at the end of a render cycle (in realtime thread).
     Vector<AudioNode*> m_nodesToDelete;
     bool m_isDeletionScheduled;