Changeset 267094 in webkit
- Timestamp:
- Sep 15, 2020 11:57:14 AM (4 years ago)
- Location:
- trunk/Source/WebCore
- Files:
-
- 3 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r267090 r267094 1 2020-09-15 Chris Dumez <cdumez@apple.com> 2 3 Improve thread-safety in ScriptProcessorNode implementation 4 https://bugs.webkit.org/show_bug.cgi?id=216555 5 6 Reviewed by Geoffrey Garen. 7 8 Improve thread-safety in ScriptProcessorNode implementation: 9 - Use a Lock instead of a volatile bool to determine if the main thread 10 is still busy firing the event. 11 - Pass the doubleBufferIndex to the main thread by capturing it in the 12 lambda instead of relying on a data member. 13 14 * Modules/webaudio/ScriptProcessorNode.cpp: 15 (WebCore::ScriptProcessorNode::ScriptProcessorNode): 16 (WebCore::ScriptProcessorNode::process): 17 (WebCore::ScriptProcessorNode::fireProcessEvent): 18 * Modules/webaudio/ScriptProcessorNode.h: 19 1 20 2020-09-15 Alex Christensen <achristensen@webkit.org> 2 21 -
trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.cpp
r266417 r267094 55 55 , ActiveDOMObject(context.scriptExecutionContext()) 56 56 , m_doubleBufferIndex(0) 57 , m_doubleBufferIndexForEvent(0)58 57 , m_bufferSize(bufferSize) 59 58 , m_bufferReadWriteIndex(0) 60 , m_isRequestOutstanding(false)61 59 , m_numberOfInputChannels(numberOfInputChannels) 62 60 , m_numberOfOutputChannels(numberOfOutputChannels) … … 186 184 // When this happens, fire an event and swap buffers. 187 185 if (!m_bufferReadWriteIndex) { 188 // Avoid building up requests on the main thread to fire process events when they're not being handled. 189 // This could be a problem if the main thread is very busy doing other things and is being held up handling previous requests. 190 if (m_isRequestOutstanding) { 191 // We're late in handling the previous request. The main thread must be very busy. 192 // The best we can do is clear out the buffer ourself here. 193 outputBuffer->zero(); 186 // Reference ourself so we don't accidentally get deleted before fireProcessEvent() gets called. 187 auto protector = makeRef(*this); 188 189 // We only wait for script code execution when the context is an offline one for performance reasons. 190 if (context().isOfflineContext()) { 191 BinarySemaphore semaphore; 192 callOnMainThread([this, &semaphore, doubleBufferIndex = m_doubleBufferIndex] { 193 fireProcessEvent(doubleBufferIndex); 194 semaphore.signal(); 195 }); 196 semaphore.wait(); 194 197 } else { 195 // Reference ourself so we don't accidentally get deleted before fireProcessEvent() gets called. 196 ref(); 197 198 // Fire the event on the main thread, not this one (which is the realtime audio thread). 199 m_doubleBufferIndexForEvent = m_doubleBufferIndex; 200 m_isRequestOutstanding = true; 201 202 // We only wait for script code execution when the context is an offline one for performance reasons. 203 if (context().isOfflineContext()) { 204 BinarySemaphore semaphore; 205 callOnMainThread([this, &semaphore] { 206 fireProcessEvent(); 207 semaphore.signal(); 208 }); 209 semaphore.wait(); 210 // De-reference to match the ref() call in process(). 211 deref(); 212 } else { 213 callOnMainThread([this] { 214 fireProcessEvent(); 215 // De-reference to match the ref() call in process(). 216 deref(); 217 }); 198 std::unique_lock<Lock> lock(m_processLock, std::try_to_lock); 199 if (!lock.owns_lock()) { 200 // We're late in handling the previous request. The main thread must be 201 // very busy. The best we can do is clear out the buffer ourself here. 202 outputBuffer->zero(); 203 return; 218 204 } 205 206 callOnMainThread([this, doubleBufferIndex = m_doubleBufferIndex, protector = WTFMove(protector)] { 207 auto locker = holdLock(m_processLock); 208 fireProcessEvent(doubleBufferIndex); 209 }); 219 210 } 220 211 … … 223 214 } 224 215 225 void ScriptProcessorNode::fireProcessEvent( )226 { 227 ASSERT(isMainThread() && m_isRequestOutstanding);216 void ScriptProcessorNode::fireProcessEvent(unsigned doubleBufferIndex) 217 { 218 ASSERT(isMainThread()); 228 219 229 bool isIndexGood = m_doubleBufferIndexForEvent< 2;220 bool isIndexGood = doubleBufferIndex < 2; 230 221 ASSERT(isIndexGood); 231 222 if (!isIndexGood) 232 223 return; 233 224 234 AudioBuffer* inputBuffer = m_inputBuffers[ m_doubleBufferIndexForEvent].get();235 AudioBuffer* outputBuffer = m_outputBuffers[ m_doubleBufferIndexForEvent].get();225 AudioBuffer* inputBuffer = m_inputBuffers[doubleBufferIndex].get(); 226 AudioBuffer* outputBuffer = m_outputBuffers[doubleBufferIndex].get(); 236 227 ASSERT(outputBuffer); 237 228 if (!outputBuffer) … … 240 231 // Avoid firing the event if the document has already gone away. 241 232 if (!context().isStopped()) { 242 // Let the audio thread know we've gotten to the point where it's OK for it to make another request.243 m_isRequestOutstanding = false;244 245 233 // Calculate playbackTime with the buffersize which needs to be processed each time when onaudioprocess is called. 246 234 // The outputBuffer being passed to JS will be played after exhausting previous outputBuffer by double-buffering. -
trunk/Source/WebCore/Modules/webaudio/ScriptProcessorNode.h
r266417 r267094 75 75 ScriptProcessorNode(BaseAudioContext&, size_t bufferSize, unsigned numberOfInputChannels, unsigned numberOfOutputChannels); 76 76 77 void fireProcessEvent( );77 void fireProcessEvent(unsigned doubleBufferIndex); 78 78 79 79 // Double buffering … … 81 81 void swapBuffers() { m_doubleBufferIndex = 1 - m_doubleBufferIndex; } 82 82 unsigned m_doubleBufferIndex; 83 unsigned m_doubleBufferIndexForEvent;84 83 Vector<RefPtr<AudioBuffer>> m_inputBuffers; 85 84 Vector<RefPtr<AudioBuffer>> m_outputBuffers; … … 87 86 size_t m_bufferSize; 88 87 unsigned m_bufferReadWriteIndex; 89 volatile bool m_isRequestOutstanding;90 88 91 89 unsigned m_numberOfInputChannels; … … 94 92 RefPtr<AudioBus> m_internalInputBus; 95 93 RefPtr<PendingActivity<ScriptProcessorNode>> m_pendingActivity; 94 Lock m_processLock; 96 95 }; 97 96
Note: See TracChangeset
for help on using the changeset viewer.