Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(126)

Unified Diff: Source/WebCore/webaudio/AudioContext.cpp

Issue 7749016: Merge 92658 - Fix thread-safety of AudioNode deletion (Closed) Base URL: http://svn.webkit.org/repository/webkit/branches/chromium/835/
Patch Set: Created 9 years, 4 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « Source/WebCore/webaudio/AudioContext.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/WebCore/webaudio/AudioContext.cpp
===================================================================
--- Source/WebCore/webaudio/AudioContext.cpp (revision 93817)
+++ Source/WebCore/webaudio/AudioContext.cpp (working copy)
@@ -119,6 +119,7 @@
, m_isAudioThreadFinished(false)
, m_document(document)
, m_destinationNode(0)
+ , m_isDeletionScheduled(false)
, m_connectionCount(0)
, m_audioThread(0)
, m_graphOwnerThread(UndefinedThreadIdentifier)
@@ -159,10 +160,6 @@
void AudioContext::constructCommon()
{
- // Note: because adoptRef() won't be called until we leave this constructor, but code in this constructor needs to reference this context,
- // relax the check.
- relaxAdoptionRequirement();
-
FFTFrame::initialize();
m_listener = AudioListener::create();
@@ -174,7 +171,7 @@
{
#if DEBUG_AUDIONODE_REFERENCES
printf("%p: AudioContext::~AudioContext()\n", this);
-#endif
+#endif
// AudioNodes keep a reference to their context, so there should be no way to be in the destructor if there are still AudioNodes around.
ASSERT(!m_nodesToDelete.size());
ASSERT(!m_referencedNodes.size());
@@ -226,7 +223,9 @@
// Get rid of the sources which may still be playing.
derefUnfinishedSourceNodes();
-
+
+ deleteMarkedNodes();
+
// Because the AudioBuffers are garbage collected, we can't delete them here.
// Instead, at least release the potentially large amount of allocated memory for the audio data.
// Note that we do this *after* the context is uninitialized and stops processing audio.
@@ -565,8 +564,9 @@
// Dynamically clean up nodes which are no longer needed.
derefFinishedSourceNodes();
- // Finally actually delete.
- deleteMarkedNodes();
+ // Don't delete in the real-time thread. Let the main thread do it.
+ // Ref-counted objects held by certain AudioNodes may not be thread-safe.
+ scheduleNodeDeletion();
// Fixup the state of any dirty AudioNodeInputs and AudioNodeOutputs.
handleDirtyAudioNodeInputs();
@@ -595,12 +595,42 @@
m_nodesToDelete.append(node);
}
+void AudioContext::scheduleNodeDeletion()
+{
+ bool isGood = m_isInitialized && isGraphOwner();
+ ASSERT(isGood);
+ if (!isGood)
+ return;
+
+ // Make sure to call deleteMarkedNodes() on main thread.
+ if (m_nodesToDelete.size() && !m_isDeletionScheduled) {
+ m_isDeletionScheduled = true;
+
+ // Don't let ourself get deleted before the callback.
+ // See matching deref() in deleteMarkedNodesDispatch().
+ ref();
+ callOnMainThread(deleteMarkedNodesDispatch, this);
+ }
+}
+
+void AudioContext::deleteMarkedNodesDispatch(void* userData)
+{
+ AudioContext* context = reinterpret_cast<AudioContext*>(userData);
+ ASSERT(context);
+ if (!context)
+ return;
+
+ context->deleteMarkedNodes();
+ context->deref();
+}
+
void AudioContext::deleteMarkedNodes()
{
- ASSERT(isGraphOwner() || isAudioThreadFinished());
+ ASSERT(isMainThread());
+ AutoLocker locker(this);
+
// Note: deleting an AudioNode can cause m_nodesToDelete to grow.
- size_t nodesDeleted = 0;
while (size_t n = m_nodesToDelete.size()) {
AudioNode* node = m_nodesToDelete[n - 1];
m_nodesToDelete.removeLast();
@@ -617,11 +647,9 @@
// Finally, delete it.
delete node;
-
- // Don't delete too many nodes per render quantum since we don't want to do too much work in the realtime audio thread.
- if (++nodesDeleted > MaxNodesToDeletePerQuantum)
- break;
}
+
+ m_isDeletionScheduled = false;
}
void AudioContext::markAudioNodeInputDirty(AudioNodeInput* input)
« no previous file with comments | « Source/WebCore/webaudio/AudioContext.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698