Title: [133239] trunk/Source/WebCore
Revision
133239
Author
crog...@google.com
Date
2012-11-01 16:23:51 -0700 (Thu, 01 Nov 2012)

Log Message

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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (133238 => 133239)


--- trunk/Source/WebCore/ChangeLog	2012-11-01 22:58:30 UTC (rev 133238)
+++ trunk/Source/WebCore/ChangeLog	2012-11-01 23:23:51 UTC (rev 133239)
@@ -1,3 +1,26 @@
+2012-11-01  Chris Rogers  <crog...@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  <rn...@webkit.org>
 
         Build fix after r133224 as suggested by Enrica.

Modified: trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp (133238 => 133239)


--- trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp	2012-11-01 22:58:30 UTC (rev 133238)
+++ trunk/Source/WebCore/Modules/webaudio/AudioContext.cpp	2012-11-01 23:23:51 UTC (rev 133239)
@@ -764,7 +764,7 @@
 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 @@
         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 @@
 
     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();

Modified: trunk/Source/WebCore/Modules/webaudio/AudioContext.h (133238 => 133239)


--- trunk/Source/WebCore/Modules/webaudio/AudioContext.h	2012-11-01 22:58:30 UTC (rev 133238)
+++ trunk/Source/WebCore/Modules/webaudio/AudioContext.h	2012-11-01 23:23:51 UTC (rev 133239)
@@ -286,6 +286,11 @@
     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;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to