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