Title: [278285] trunk/Source/WebCore
Revision
278285
Author
[email protected]
Date
2021-05-31 17:03:36 -0700 (Mon, 31 May 2021)

Log Message

Fix thread safety issues in OscillatorNode
https://bugs.webkit.org/show_bug.cgi?id=226450

Reviewed by Darin Adler.

Adopt thread safety annotations in OscillatorNode and fix bugs found by clang.
In particular, propagatesSilence() was failing to grab the lock before accessing
m_periodicWave, which gets modified on the main thread.

* Modules/webaudio/OscillatorNode.cpp:
(WebCore::OscillatorNode::create):
(WebCore::OscillatorNode::setTypeForBindings):
(WebCore::OscillatorNode::propagatesSilence const):
* Modules/webaudio/OscillatorNode.h:
* Modules/webaudio/OscillatorNode.idl:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (278284 => 278285)


--- trunk/Source/WebCore/ChangeLog	2021-06-01 00:03:03 UTC (rev 278284)
+++ trunk/Source/WebCore/ChangeLog	2021-06-01 00:03:36 UTC (rev 278285)
@@ -1,5 +1,23 @@
 2021-05-31  Chris Dumez  <[email protected]>
 
+        Fix thread safety issues in OscillatorNode
+        https://bugs.webkit.org/show_bug.cgi?id=226450
+
+        Reviewed by Darin Adler.
+
+        Adopt thread safety annotations in OscillatorNode and fix bugs found by clang.
+        In particular, propagatesSilence() was failing to grab the lock before accessing
+        m_periodicWave, which gets modified on the main thread.
+
+        * Modules/webaudio/OscillatorNode.cpp:
+        (WebCore::OscillatorNode::create):
+        (WebCore::OscillatorNode::setTypeForBindings):
+        (WebCore::OscillatorNode::propagatesSilence const):
+        * Modules/webaudio/OscillatorNode.h:
+        * Modules/webaudio/OscillatorNode.idl:
+
+2021-05-31  Chris Dumez  <[email protected]>
+
         Fix thread safety issues in PannerNode
         https://bugs.webkit.org/show_bug.cgi?id=226455
 

Modified: trunk/Source/WebCore/Modules/webaudio/OscillatorNode.cpp (278284 => 278285)


--- trunk/Source/WebCore/Modules/webaudio/OscillatorNode.cpp	2021-06-01 00:03:03 UTC (rev 278284)
+++ trunk/Source/WebCore/Modules/webaudio/OscillatorNode.cpp	2021-06-01 00:03:36 UTC (rev 278285)
@@ -72,7 +72,7 @@
     if (options.periodicWave)
         oscillator->setPeriodicWave(*options.periodicWave);
     else {
-        result = oscillator->setType(options.type);
+        result = oscillator->setTypeForBindings(options.type);
         if (result.hasException())
             return result.releaseException();
     }
@@ -97,9 +97,10 @@
     uninitialize();
 }
 
-ExceptionOr<void> OscillatorNode::setType(OscillatorType type)
+ExceptionOr<void> OscillatorNode::setTypeForBindings(OscillatorType type)
 {
     ALWAYS_LOG(LOGIDENTIFIER, type);
+    ASSERT(isMainThread());
 
     if (type == OscillatorType::Custom) {
         if (m_type != OscillatorType::Custom)
@@ -438,7 +439,13 @@
 
 bool OscillatorNode::propagatesSilence() const
 {
-    return !isPlayingOrScheduled() || hasFinished() || !m_periodicWave.get();
+    ASSERT(context().isAudioThread());
+    if (!isPlayingOrScheduled() || hasFinished())
+        return true;
+    if (!m_processLock.tryLock())
+        return false; // Assume we have a periodic wave if we are unable to grab the lock.
+    Locker locker { AdoptLock, m_processLock };
+    return !m_periodicWave.get();
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/Modules/webaudio/OscillatorNode.h (278284 => 278285)


--- trunk/Source/WebCore/Modules/webaudio/OscillatorNode.h	2021-06-01 00:03:03 UTC (rev 278284)
+++ trunk/Source/WebCore/Modules/webaudio/OscillatorNode.h	2021-06-01 00:03:36 UTC (rev 278285)
@@ -44,8 +44,8 @@
 
     const char* activeDOMObjectName() const final { return "OscillatorNode"; }
 
-    OscillatorType type() const { return m_type; }
-    ExceptionOr<void> setType(OscillatorType);
+    OscillatorType typeForBindings() const { ASSERT(isMainThread()); return m_type; }
+    ExceptionOr<void> setTypeForBindings(OscillatorType);
 
     AudioParam* frequency() { return m_frequency.get(); }
     AudioParam* detune() { return m_detune.get(); }
@@ -62,15 +62,15 @@
     double latencyTime() const final { return 0; }
 
     // Returns true if there are sample-accurate timeline parameter changes.
-    bool calculateSampleAccuratePhaseIncrements(size_t framesToProcess);
+    bool calculateSampleAccuratePhaseIncrements(size_t framesToProcess) WTF_REQUIRES_LOCK(m_processLock);
 
-    double processARate(int, float* destP, double virtualReadIndex, float* phaseIncrements);
-    double processKRate(int, float* destP, double virtualReadIndex);
+    double processARate(int, float* destP, double virtualReadIndex, float* phaseIncrements) WTF_REQUIRES_LOCK(m_processLock);
+    double processKRate(int, float* destP, double virtualReadIndex) WTF_REQUIRES_LOCK(m_processLock);
 
     bool propagatesSilence() const final;
 
     // One of the waveform types defined in the enum.
-    OscillatorType m_type;
+    OscillatorType m_type; // Only used on the main thread.
     
     // Frequency value in Hertz.
     RefPtr<AudioParam> m_frequency;
@@ -91,7 +91,7 @@
     AudioFloatArray m_phaseIncrements;
     AudioFloatArray m_detuneValues;
     
-    RefPtr<PeriodicWave> m_periodicWave;
+    RefPtr<PeriodicWave> m_periodicWave WTF_GUARDED_BY_LOCK(m_processLock);
 };
 
 String convertEnumerationToString(OscillatorType); // In JSOscillatorNode.cpp

Modified: trunk/Source/WebCore/Modules/webaudio/OscillatorNode.idl (278284 => 278285)


--- trunk/Source/WebCore/Modules/webaudio/OscillatorNode.idl	2021-06-01 00:03:03 UTC (rev 278284)
+++ trunk/Source/WebCore/Modules/webaudio/OscillatorNode.idl	2021-06-01 00:03:36 UTC (rev 278285)
@@ -31,7 +31,7 @@
 ] interface OscillatorNode : AudioScheduledSourceNode {
     [EnabledBySetting=WebAudio] constructor (BaseAudioContext context, optional OscillatorOptions options);
 
-    attribute OscillatorType type;
+    [ImplementedAs=typeForBindings] attribute OscillatorType type;
 
     readonly attribute AudioParam frequency; // in Hertz
     readonly attribute AudioParam detune; // in Cents
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to