Fix threading races on AudioDSPKernelProcessor::m_kernels

Source/core/platform/audio/AudioDSPKernelProcessor.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 32 matching lines @@
     : AudioProcessor(sampleRate, numberOfChannels)
     , m_hasJustReset(true)
 {
 }
 
 void AudioDSPKernelProcessor::initialize()
 {
     if (isInitialized())
         return;
 
+    MutexLocker locker(m_processLock);
     ASSERT(!m_kernels.size());
 
     // Create processing kernels, one per channel.
     for (unsigned i = 0; i < numberOfChannels(); ++i)
         m_kernels.append(createKernel());
 
     m_initialized = true;
     m_hasJustReset = true;
 }
 
 void AudioDSPKernelProcessor::uninitialize()
 {
     if (!isInitialized())
         return;
 
+    MutexLocker locker(m_processLock);
     m_kernels.clear();
 
     m_initialized = false;
 }
 
 void AudioDSPKernelProcessor::process(const AudioBus* source, AudioBus* destination, size_t framesToProcess)
 {
     ASSERT(source && destination);
     if (!source || !destination)
         return;
 
     if (!isInitialized()) {
         destination->zero();
         return;
     }
 
-    bool channelCountMatches = source->numberOfChannels() == destination->numberOfChannels() && source->numberOfChannels() == m_kernels.size();
-    ASSERT(channelCountMatches);
-    if (!channelCountMatches)
-        return;
+    MutexTryLocker tryLocker(m_processLock);
+    if (tryLocker.locked()) {
+        bool channelCountMatches = source->numberOfChannels() == destination->numberOfChannels() && source->numberOfChannels() == m_kernels.size();
+        ASSERT(channelCountMatches);
+        if (!channelCountMatches)
+            return;
 
-    for (unsigned i = 0; i < m_kernels.size(); ++i)
-        m_kernels[i]->process(source->channel(i)->data(), destination->channel(i)->mutableData(), framesToProcess);
+        for (unsigned i = 0; i < m_kernels.size(); ++i)
+            m_kernels[i]->process(source->channel(i)->data(), destination->channel(i)->mutableData(), framesToProcess);
+    } else {
+        // Unfortunately, the kernel is being processed by another thread.
+        // See also ConvolverNode::process().
+        destination->zero();
+    }
 }
 
 // Resets filter state
 void AudioDSPKernelProcessor::reset()

[Ken Russell (switch to Gerrit), 2013/09/04 18:18:46]

Please add an assertion that this is called on the main thread -- and run tests
in a debug build.

[haraken, 2013/09/04 19:50:05]

Done.

 {
     if (!isInitialized())
         return;
 
     // Forces snap to parameter values - first time.
     // Any processing depending on this value must set it to false at the appropriate time.
     m_hasJustReset = true;
 
+    MutexLocker locker(m_processLock);
     for (unsigned i = 0; i < m_kernels.size(); ++i)
         m_kernels[i]->reset();
 }
 
 void AudioDSPKernelProcessor::setNumberOfChannels(unsigned numberOfChannels)
 {
     if (numberOfChannels == m_numberOfChannels)
         return;
 
     ASSERT(!isInitialized());
     if (!isInitialized())
         m_numberOfChannels = numberOfChannels;
 }
 
 double AudioDSPKernelProcessor::tailTime() const
 {
+    MutexLocker locker(m_processLock);

[Ken Russell (switch to Gerrit), 2013/09/04 18:18:46]

I believe this is called on the audio thread and therefore must use a trylock.

Please add an assertion like ASSERT(context()->isAudioThread()).

[haraken, 2013/09/04 19:50:05]

Done.

     // It is expected that all the kernels have the same tailTime.
     return !m_kernels.isEmpty() ? m_kernels.first()->tailTime() : 0;
 }
 
 double AudioDSPKernelProcessor::latencyTime() const
 {
+    MutexLocker locker(m_processLock);

[Ken Russell (switch to Gerrit), 2013/09/04 18:18:46]

trylock needed here too.

[haraken, 2013/09/04 19:50:05]

Done.

     // It is expected that all the kernels have the same latencyTime.
     return !m_kernels.isEmpty() ? m_kernels.first()->latencyTime() : 0;
 }
 
 } // namespace WebCore
 
 #endif // ENABLE(WEB_AUDIO)