Modified: trunk/Source/WebCore/ChangeLog (92657 => 92658)
--- trunk/Source/WebCore/ChangeLog 2011-08-09 00:25:39 UTC (rev 92657)
+++ trunk/Source/WebCore/ChangeLog 2011-08-09 00:49:04 UTC (rev 92658)
@@ -1,3 +1,23 @@
+2011-08-08 Chris Rogers <[email protected]>
+
+ Fix thread-safety of AudioNode deletion
+ https://bugs.webkit.org/show_bug.cgi?id=65888
+
+ Reviewed by Kenneth Russell
+
+ No new tests - _javascript_ API is not affected.
+
+ * webaudio/AudioContext.cpp:
+ (WebCore::AudioContext::AudioContext):
+ (WebCore::AudioContext::constructCommon):
+ (WebCore::AudioContext::~AudioContext):
+ (WebCore::AudioContext::uninitialize):
+ (WebCore::AudioContext::handlePostRenderTasks):
+ (WebCore::AudioContext::scheduleNodeDeletion):
+ (WebCore::AudioContext::deleteMarkedNodesDispatch):
+ (WebCore::AudioContext::deleteMarkedNodes):
+ * webaudio/AudioContext.h:
+
2011-08-08 Chris Marrin <[email protected]>
Fix build breakage caused by http://trac.webkit.org/changeset/92651
Modified: trunk/Source/WebCore/webaudio/AudioContext.cpp (92657 => 92658)
--- trunk/Source/WebCore/webaudio/AudioContext.cpp 2011-08-09 00:25:39 UTC (rev 92657)
+++ trunk/Source/WebCore/webaudio/AudioContext.cpp 2011-08-09 00:49:04 UTC (rev 92658)
@@ -120,6 +120,7 @@
, m_isAudioThreadFinished(false)
, m_document(document)
, m_destinationNode(0)
+ , m_isDeletionScheduled(false)
, m_connectionCount(0)
, m_audioThread(0)
, m_graphOwnerThread(UndefinedThreadIdentifier)
@@ -160,10 +161,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();
@@ -175,7 +172,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());
@@ -227,7 +224,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.
@@ -566,8 +565,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();
@@ -596,12 +596,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();
@@ -618,11 +648,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)
Modified: trunk/Source/WebCore/webaudio/AudioContext.h (92657 => 92658)
--- trunk/Source/WebCore/webaudio/AudioContext.h 2011-08-09 00:25:39 UTC (rev 92657)
+++ trunk/Source/WebCore/webaudio/AudioContext.h 2011-08-09 00:49:04 UTC (rev 92658)
@@ -38,6 +38,7 @@
#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
#include <wtf/RefPtr.h>
+#include <wtf/ThreadSafeRefCounted.h>
#include <wtf/Threading.h>
#include <wtf/Vector.h>
#include <wtf/text/AtomicStringHash.h>
@@ -67,7 +68,7 @@
// AudioContext is the cornerstone of the web audio API and all AudioNodes are created from it.
// For thread safety between the audio thread and the main thread, it has a rendering graph locking mechanism.
-class AudioContext : public ActiveDOMObject, public RefCounted<AudioContext>, public EventTarget {
+class AudioContext : public ActiveDOMObject, public ThreadSafeRefCounted<AudioContext>, public EventTarget {
public:
// Create an AudioContext for rendering to the audio hardware.
static PassRefPtr<AudioContext> create(Document*);
@@ -136,10 +137,10 @@
// Called periodically at the end of each render quantum to dereference finished source nodes.
void derefFinishedSourceNodes();
- // We reap all marked nodes at the end of each realtime render quantum in deleteMarkedNodes().
+ // We schedule deletion of all marked nodes at the end of each realtime render quantum.
void markForDeletion(AudioNode*);
void deleteMarkedNodes();
-
+
// Keeps track of the number of connections made.
void incrementConnectionCount()
{
@@ -209,9 +210,9 @@
DEFINE_ATTRIBUTE_EVENT_LISTENER(complete);
- // Reconcile ref/deref which are defined both in AudioNode and EventTarget.
- using RefCounted<AudioContext>::ref;
- using RefCounted<AudioContext>::deref;
+ // Reconcile ref/deref which are defined both in ThreadSafeRefCounted and EventTarget.
+ using ThreadSafeRefCounted<AudioContext>::ref;
+ using ThreadSafeRefCounted<AudioContext>::deref;
void startRendering();
void fireCompletionEvent();
@@ -225,6 +226,9 @@
void lazyInitialize();
void uninitialize();
+
+ void scheduleNodeDeletion();
+ static void deleteMarkedNodesDispatch(void* userData);
bool m_isInitialized;
bool m_isAudioThreadFinished;
@@ -257,8 +261,10 @@
// Either accessed when the graph lock is held, or on the main thread when the audio thread has finished.
Vector<AudioNode*> m_referencedNodes;
- // Accumulate nodes which need to be deleted at the end of a render cycle (in realtime thread) here.
+ // Accumulate nodes which need to be deleted here.
+ // 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;
// Only accessed when the graph lock is held.
HashSet<AudioNodeInput*> m_dirtyAudioNodeInputs;
