WebAudio: Fix AudioNode leak in a case that AudioNode is not disconnected from the graph explicitly.

Source/modules/webaudio/AudioContext.cpp

Index: Source/modules/webaudio/AudioContext.cpp
diff --git a/Source/modules/webaudio/AudioContext.cpp b/Source/modules/webaudio/AudioContext.cpp
index 1b0f81e320ad2f63d9e4d1864f782a2afa7b386a..136a9b7ea1eb500bad614349f7a900b03add60fe 100644
--- a/Source/modules/webaudio/AudioContext.cpp
+++ b/Source/modules/webaudio/AudioContext.cpp
@@ -109,6 +109,8 @@ AudioContext::AudioContext(Document* document)
     , m_connectionCount(0)
     , m_didInitializeContextGraphMutex(false)
     , m_audioThread(0)
+    , m_lastZombie(nullptr)
+    , m_lastRemovableZombie(nullptr)
     , m_isOfflineContext(false)
     , m_contextState(Suspended)
     , m_cachedSampleFrame(0)
@@ -134,6 +136,8 @@ AudioContext::AudioContext(Document* document, unsigned numberOfChannels, size_t
     , m_connectionCount(0)
     , m_didInitializeContextGraphMutex(false)
     , m_audioThread(0)
+    , m_lastZombie(nullptr)
+    , m_lastRemovableZombie(nullptr)
     , m_isOfflineContext(true)
     , m_contextState(Suspended)
     , m_cachedSampleFrame(0)
@@ -162,6 +166,7 @@ AudioContext::~AudioContext()
     ASSERT(!m_renderingAutomaticPullNodes.size());
     ASSERT(!m_suspendResolvers.size());
     ASSERT(!m_resumeResolvers.size());
+    ThreadState::current()->removeMarkingTask(this);

[haraken, 2015/02/06 07:57:23]

Can we move this to AudioContext::uninitialize()?

[tkent, 2015/02/09 07:00:50]

Yes.  It's consistent because we call addMarkingTask in initialize().  Done.

We need to call purifyZombies() before a marking after the last
AudioContext::handlePostRenderTasks().  uninitialize().  So, we also need to add
purifyZombies() to uninitialize().

 }
 
 void AudioContext::initialize()
@@ -169,6 +174,7 @@ void AudioContext::initialize()
     if (isInitialized())
         return;
 
+    ThreadState::current()->addMarkingTask(this);
     FFTFrame::initialize();
     m_listener = AudioListener::create();
 
@@ -817,6 +823,8 @@ void AudioContext::handlePostRenderTasks()
         // Dynamically clean up nodes which are no longer needed.
         derefFinishedSourceNodes();
 
+        m_lastRemovableZombie = m_lastZombie;
+
         // Fixup the state of any dirty AudioSummingJunctions and AudioNodeOutputs.
         handleDirtyAudioSummingJunctions();
         handleDirtyAudioNodeOutputs();
@@ -1110,14 +1118,7 @@ void AudioContext::trace(Visitor* visitor)
     visitor->trace(m_renderTarget);
     visitor->trace(m_destinationNode);
     visitor->trace(m_listener);
-    // trace() can be called in AudioContext constructor, and
-    // m_contextGraphMutex might be unavailable.
-    if (m_didInitializeContextGraphMutex) {
-        AutoLocker lock(this);
-        visitor->trace(m_referencedNodes);
-    } else {
-        visitor->trace(m_referencedNodes);
-    }
+    visitor->trace(m_referencedNodes);
     visitor->trace(m_resumeResolvers);
     visitor->trace(m_suspendResolvers);
     visitor->trace(m_liveNodes);
@@ -1155,6 +1156,29 @@ SecurityOrigin* AudioContext::securityOrigin() const
     return executionContext()->securityOrigin();
 }
 
+void AudioContext::setLastZombie(void* object)
+{
+    ASSERT(isGraphOwner());
+    m_lastZombie = object;
+}
+
+void AudioContext::willStartMarking(ThreadState& threadState)

[haraken, 2015/02/06 07:57:23]

willStartMarking is called not only in the ZombieMarking but also in a normal
global marking. Is it OK (if so, why)?

[tkent, 2015/02/09 07:00:50]

It's ok.
There are two purposes of this hook:

* Simplify AudioContext::trace by calling lock()/unlock() in the task
  We need this for both of global marking and zombie marking.

* Calls purifyZombies() before zombie marking
  purifyZombies() on one of the markings is enough, and purifyZombies() on both
of markings is harmless.   The current code applies the latter for code
simplicity.

+{
+    lock();

[haraken, 2015/02/06 07:57:23]

What happens if a GC is triggered during AudioContext's constructor before the
lock gets initialized?

[tkent, 2015/02/09 07:00:50]

It won't happen.  This task is registered after the mutex initialization.

+    if (!m_lastRemovableZombie)
+        return;
+    if (m_lastZombie != m_lastRemovableZombie)

[haraken, 2015/02/06 07:57:23]

I don't quite get what m_lastZombie and m_lastRemovableZombie point to. Would
you elaborate?

[tkent, 2015/02/09 07:00:50]

We need to avoid the following scenario:

1. GC happens
2. Some AudioNodes are unreachable, and their weak processing makes them
zombies.
3. Another GC happens.  We can't call purifyZombies() yet because
AudioContext::handlePostRenderTasks() is not called yet.
4. AudioContext::handlePostRenderTasks() is called on audio thread.  We may
purify all existing zombies, but we can't purify immediately because we're on
audio thread.
5. Another GC happens.  More AudioNodes are unreachable and their weak
processing adds new zombies.
6. Now, we can purify zombies made at step 2, but shouldn't purify zombies made
at step 5.

There are some ways to resolve this issue.  The current code would be the
simplest one.

+        return;
+    threadState.purifyZombies();
+    m_lastZombie = nullptr;
+    m_lastRemovableZombie = nullptr;
+}
+
+void AudioContext::didFinishMarking(ThreadState&)
+{
+    unlock();
+}
+
 } // namespace blink
 
 #endif // ENABLE(WEB_AUDIO)