Changeset 293273 in webkit
- Timestamp:
- Apr 22, 2022, 5:37:40 PM (4 years ago)
- Location:
- branches/safari-613.2.7.1-branch/Source/WebKit
- Files:
-
- 5 edited
-
ChangeLog (modified) (1 diff)
-
GPUProcess/GPUConnectionToWebProcess.cpp (modified) (2 diffs)
-
GPUProcess/GPUConnectionToWebProcess.h (modified) (1 diff)
-
GPUProcess/webrtc/LibWebRTCCodecsProxy.h (modified) (3 diffs)
-
GPUProcess/webrtc/LibWebRTCCodecsProxy.mm (modified) (8 diffs)
Legend:
- Unmodified
- Added
- Removed
-
branches/safari-613.2.7.1-branch/Source/WebKit/ChangeLog
r293270 r293273 1 2022-04-22 Russell Epstein <repstein@apple.com>2 3 Apply patch. rdar://problem/889041604 5 2022-04-22 Kimmo Kinnunen <kkinnunen@apple.com>6 7 Multiple concurrency violations in LibWebRTCCodecsProxy8 https://bugs.webkit.org/show_bug.cgi?id=2367679 <rdar://88904160>10 11 Reviewed by Antti Koivisto.12 13 - ThreadMessageReceivers should not add IPC listeners in constructors,14 as the delivery starts right away and uses the unconstructed virtual pointer.15 - The work queue functions should not use GPUConnectionToWebProcess, as that is16 main thread object.17 - Locked m_encoders, m_decoders are sometimes accessed without lock.18 19 Instead:20 - Add the IPC listeners in initialize function.21 - Remove the IPC listeners when GPUConnectionToWebProcess disconnects.22 - Store the thread-safe conection, video frame object heap, process identity23 objects as member variables.24 - Do not lock m_encoders, m_decoders. If they are work queue instances,25 just access them in the work queue functions. Add thread requirements26 to the variables so that the compiler checks the access.27 - Use IPC testing assertions when skipping incorrect messages.28 - Use separate atomic counter (bool) to check if allowsExitUnderMemoryPressure.29 30 No new tests, tested with existing tests and ASAN.31 32 * GPUProcess/GPUConnectionToWebProcess.cpp:33 (WebKit::GPUConnectionToWebProcess::~GPUConnectionToWebProcess):34 (WebKit::GPUConnectionToWebProcess::didClose):35 * GPUProcess/GPUConnectionToWebProcess.h:36 * GPUProcess/webrtc/LibWebRTCCodecsProxy.h:37 * GPUProcess/webrtc/LibWebRTCCodecsProxy.mm:38 (WebKit::LibWebRTCCodecsProxy::create):39 (WebKit::LibWebRTCCodecsProxy::LibWebRTCCodecsProxy):40 (WebKit::LibWebRTCCodecsProxy::stopListeningForIPC):41 (WebKit::LibWebRTCCodecsProxy::initialize):42 (WebKit::LibWebRTCCodecsProxy::dispatchToThread):43 (WebKit::LibWebRTCCodecsProxy::createDecoderCallback):44 (WebKit::LibWebRTCCodecsProxy::createH264Decoder):45 (WebKit::LibWebRTCCodecsProxy::createH265Decoder):46 (WebKit::LibWebRTCCodecsProxy::createVP9Decoder):47 (WebKit::LibWebRTCCodecsProxy::releaseDecoder):48 (WebKit::LibWebRTCCodecsProxy::createEncoder):49 (WebKit::LibWebRTCCodecsProxy::releaseEncoder):50 (WebKit::LibWebRTCCodecsProxy::initializeEncoder):51 (WebKit::LibWebRTCCodecsProxy::findEncoder):52 (WebKit::LibWebRTCCodecsProxy::encodeFrame):53 (WebKit::LibWebRTCCodecsProxy::setEncodeRates):54 (WebKit::LibWebRTCCodecsProxy::setSharedVideoFrameSemaphore):55 (WebKit::LibWebRTCCodecsProxy::setSharedVideoFrameMemory):56 (WebKit::LibWebRTCCodecsProxy::allowsExitUnderMemoryPressure const):57 58 1 2022-04-22 Russell Epstein <repstein@apple.com> 59 2 -
branches/safari-613.2.7.1-branch/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.cpp
r293270 r293273 275 275 m_sampleBufferDisplayLayerManager->close(); 276 276 #endif 277 #if PLATFORM(COCOA) && USE(LIBWEBRTC) 278 m_libWebRTCCodecsProxy->close(); 279 #endif 277 280 278 281 --gObjectCountForTesting; … … 307 310 }); 308 311 #endif 309 #if PLATFORM(COCOA) && USE(LIBWEBRTC) 310 m_libWebRTCCodecsProxy = nullptr; 311 #endif 312 312 313 gpuProcess().connectionToWebProcessClosed(connection); 313 314 gpuProcess().removeGPUConnectionToWebProcess(*this); // May destroy |this|. -
branches/safari-613.2.7.1-branch/Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h
r293270 r293273 286 286 PAL::SessionID m_sessionID; 287 287 #if PLATFORM(COCOA) && USE(LIBWEBRTC) 288 IPC::ScopedActiveMessageReceiveQueue<LibWebRTCCodecsProxy> m_libWebRTCCodecsProxy;288 Ref<LibWebRTCCodecsProxy> m_libWebRTCCodecsProxy; 289 289 #endif 290 290 #if PLATFORM(COCOA) && ENABLE(MEDIA_STREAM) -
branches/safari-613.2.7.1-branch/Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.h
r293270 r293273 32 32 #include "RTCDecoderIdentifier.h" 33 33 #include "RTCEncoderIdentifier.h" 34 #include <atomic> 35 #include <wtf/ThreadAssertions.h> 34 #include <wtf/Lock.h> 36 35 37 36 namespace IPC { … … 53 52 class GPUConnectionToWebProcess; 54 53 55 class LibWebRTCCodecsProxy final: public IPC::Connection::ThreadMessageReceiverRefCounted {54 class LibWebRTCCodecsProxy : public IPC::Connection::ThreadMessageReceiverRefCounted { 56 55 WTF_MAKE_FAST_ALLOCATED; 57 56 public: 58 static Ref<LibWebRTCCodecsProxy> create(GPUConnectionToWebProcess& );57 static Ref<LibWebRTCCodecsProxy> create(GPUConnectionToWebProcess& process) { return adoptRef(*new LibWebRTCCodecsProxy(process)); } 59 58 ~LibWebRTCCodecsProxy(); 60 void stopListeningForIPC(Ref<LibWebRTCCodecsProxy>&& refFromConnection); 59 60 void close(); 61 61 62 bool allowsExitUnderMemoryPressure() const; 62 63 63 64 private: 64 65 explicit LibWebRTCCodecsProxy(GPUConnectionToWebProcess&); 65 void initialize();66 auto createDecoderCallback(RTCDecoderIdentifier, bool useRemoteFrames);67 WorkQueue& workQueue() const { return m_queue; }68 66 69 67 // IPC::Connection::ThreadMessageReceiver … … 88 86 CFDictionaryRef ioSurfacePixelBufferCreationOptions(IOSurfaceRef); 89 87 88 GPUConnectionToWebProcess& m_gpuConnectionToWebProcess; 89 90 mutable Lock m_lock; 91 HashMap<RTCDecoderIdentifier, webrtc::LocalDecoder> m_decoders WTF_GUARDED_BY_LOCK(m_lock); // Only modified on the libWebRTCCodecsQueue but may get accessed from the main thread. 92 HashMap<RTCEncoderIdentifier, webrtc::LocalEncoder> m_encoders WTF_GUARDED_BY_LOCK(m_lock); // Only modified on the libWebRTCCodecsQueue but may get accessed from the main thread. 90 93 91 94 Ref<WorkQueue> m_queue; 92 HashMap<RTCDecoderIdentifier, webrtc::LocalDecoder> m_decoders WTF_GUARDED_BY_LOCK(workQueue());93 HashMap<RTCEncoderIdentifier, webrtc::LocalEncoder> m_encoders WTF_GUARDED_BY_LOCK(workQueue());94 std::atomic<bool> m_hasEncodersOrDecoders { false };95 95 }; 96 96 -
branches/safari-613.2.7.1-branch/Source/WebKit/GPUProcess/webrtc/LibWebRTCCodecsProxy.mm
r293270 r293273 31 31 #import "GPUConnectionToWebProcess.h" 32 32 #import "GPUProcess.h" 33 #import "IPCTester.h"34 33 #import "LibWebRTCCodecsMessages.h" 35 34 #import "LibWebRTCCodecsProxyMessages.h" … … 45 44 namespace WebKit { 46 45 47 Ref<LibWebRTCCodecsProxy> LibWebRTCCodecsProxy::create(GPUConnectionToWebProcess& webProcessConnection)48 { 49 auto instance = adoptRef(*new LibWebRTCCodecsProxy(webProcessConnection));50 instance->initialize(); 51 return instance;52 } 53 54 LibWebRTCCodecsProxy:: LibWebRTCCodecsProxy(GPUConnectionToWebProcess& webProcessConnection)55 : m_connection(webProcessConnection.connection()) 56 , m_queue(webProcessConnection.gpuProcess().libWebRTCCodecsQueue()) 57 , m_resourceOwner(webProcessConnection.webProcessIdentity()) 58 { 59 } 60 61 LibWebRTCCodecsProxy::~LibWebRTCCodecsProxy() = default; 62 63 void LibWebRTCCodecsProxy:: stopListeningForIPC(Ref<LibWebRTCCodecsProxy>&& refFromConnection)64 { 65 m_ connection->removeThreadMessageReceiver(Messages::LibWebRTCCodecsProxy::messageReceiverName());66 67 dispatchToThread([this, protectedThis = WTFMove(refFromConnection)] {68 assertIsCurrent(workQueue());46 LibWebRTCCodecsProxy::LibWebRTCCodecsProxy(GPUConnectionToWebProcess& connection) 47 : m_gpuConnectionToWebProcess(connection) 48 , m_queue(connection.gpuProcess().libWebRTCCodecsQueue()) 49 { 50 m_gpuConnectionToWebProcess.connection().addThreadMessageReceiver(Messages::LibWebRTCCodecsProxy::messageReceiverName(), this); 51 } 52 53 LibWebRTCCodecsProxy::~LibWebRTCCodecsProxy() 54 { 55 } 56 57 void LibWebRTCCodecsProxy::dispatchToThread(Function<void()>&& function) 58 { 59 m_queue->dispatch(WTFMove(function)); 60 } 61 62 void LibWebRTCCodecsProxy::close() 63 { 64 m_gpuConnectionToWebProcess.connection().removeThreadMessageReceiver(Messages::LibWebRTCCodecsProxy::messageReceiverName()); 65 66 dispatchToThread([this, protectedThis = Ref { *this }] { 67 Locker locker { m_lock }; 69 68 auto decoders = WTFMove(m_decoders); 70 69 for (auto decoder : decoders.values()) … … 76 75 } 77 76 78 void LibWebRTCCodecsProxy::initialize() 79 { 80 m_connection->addThreadMessageReceiver(Messages::LibWebRTCCodecsProxy::messageReceiverName(), this); 81 } 82 83 void LibWebRTCCodecsProxy::dispatchToThread(Function<void()>&& function) 84 { 85 m_queue->dispatch(WTFMove(function)); 86 } 87 88 auto LibWebRTCCodecsProxy::createDecoderCallback(RTCDecoderIdentifier identifier) 89 { 90 return [identifier, connection = m_connection, resourceOwner = m_resourceOwner] (CVPixelBufferRef pixelBuffer, uint32_t timeStampNs, uint32_t timeStamp) mutable { 91 auto sample = WebCore::MediaSampleAVFObjC::createImageSample(pixelBuffer, WebCore::MediaSample::VideoRotation::None, false, MediaTime(timeStampNs, 1), { }); 92 if (!sample) 93 return; 94 if (resourceOwner) 95 sample->setOwnershipIdentity(resourceOwner); 96 connection->send(Messages::LibWebRTCCodecs::CompletedDecoding { identifier, timeStamp, *sample }, 0); 77 static Function<void(CVPixelBufferRef pixelBuffer, uint32_t timeStampNs, uint32_t timeStamp)> createDecoderCallback(RTCDecoderIdentifier identifier, GPUConnectionToWebProcess& gpuConnectionToWebProcess) 78 { 79 return [connection = Ref { gpuConnectionToWebProcess.connection() }, resourceOwner = gpuConnectionToWebProcess.webProcessIdentity(), identifier] (CVPixelBufferRef pixelBuffer, uint32_t timeStampNs, uint32_t timeStamp) { 80 if (auto sample = WebCore::RemoteVideoSample::create(pixelBuffer, MediaTime(timeStampNs, 1))) { 81 if (resourceOwner) 82 sample->setOwnershipIdentity(resourceOwner); 83 connection->send(Messages::LibWebRTCCodecs::CompletedDecoding { identifier, timeStamp, *sample }, 0); 84 } 97 85 }; 98 86 } … … 100 88 void LibWebRTCCodecsProxy::createH264Decoder(RTCDecoderIdentifier identifier) 101 89 { 102 assertIsCurrent(workQueue());103 auto result = m_decoders.add(identifier, webrtc::createLocalH264Decoder(makeBlockPtr(createDecoderCallback(identifier)).get()));104 ASSERT _UNUSED(result, result.isNewEntry || isTestingIPC());105 m_ hasEncodersOrDecoders = true;90 ASSERT(!isMainRunLoop()); 91 Locker locker { m_lock }; 92 ASSERT(!m_decoders.contains(identifier)); 93 m_decoders.add(identifier, webrtc::createLocalH264Decoder(makeBlockPtr(createDecoderCallback(identifier, m_gpuConnectionToWebProcess)).get())); 106 94 } 107 95 108 96 void LibWebRTCCodecsProxy::createH265Decoder(RTCDecoderIdentifier identifier) 109 97 { 110 assertIsCurrent(workQueue());111 auto result = m_decoders.add(identifier, webrtc::createLocalH265Decoder(makeBlockPtr(createDecoderCallback(identifier)).get()));112 ASSERT _UNUSED(result, result.isNewEntry || isTestingIPC());113 m_ hasEncodersOrDecoders = true;98 ASSERT(!isMainRunLoop()); 99 Locker locker { m_lock }; 100 ASSERT(!m_decoders.contains(identifier)); 101 m_decoders.add(identifier, webrtc::createLocalH265Decoder(makeBlockPtr(createDecoderCallback(identifier, m_gpuConnectionToWebProcess)).get())); 114 102 } 115 103 116 104 void LibWebRTCCodecsProxy::createVP9Decoder(RTCDecoderIdentifier identifier) 117 105 { 118 assertIsCurrent(workQueue());119 auto result = m_decoders.add(identifier, webrtc::createLocalVP9Decoder(makeBlockPtr(createDecoderCallback(identifier)).get()));120 ASSERT _UNUSED(result, result.isNewEntry || isTestingIPC());121 m_ hasEncodersOrDecoders = true;106 ASSERT(!isMainRunLoop()); 107 Locker locker { m_lock }; 108 ASSERT(!m_decoders.contains(identifier)); 109 m_decoders.add(identifier, webrtc::createLocalVP9Decoder(makeBlockPtr(createDecoderCallback(identifier, m_gpuConnectionToWebProcess)).get())); 122 110 } 123 111 124 112 void LibWebRTCCodecsProxy::releaseDecoder(RTCDecoderIdentifier identifier) 125 113 { 126 assertIsCurrent(workQueue());127 auto decoder = m_decoders.take(identifier);128 if (!decoder) {129 ASSERT_IS_TESTING_IPC();130 return;131 }132 webrtc::releaseLocalDecoder(decoder); 133 m_hasEncodersOrDecoders = !m_encoders.isEmpty() || !m_decoders.isEmpty();} 134 } 135 136 void LibWebRTCCodecsProxy::decodeFrame(RTCDecoderIdentifier identifier, uint32_t timeStamp, const IPC::DataReference& data) 137 { 138 assertIsCurrent(workQueue());114 ASSERT(!isMainRunLoop()); 115 Locker locker { m_lock }; 116 ASSERT(m_decoders.contains(identifier)); 117 if (auto decoder = m_decoders.take(identifier)) 118 webrtc::releaseLocalDecoder(decoder); 119 } 120 121 // For performance reasons, this function accesses m_decoders without locking. This is safe because this function runs on the libWebRTCCodecsQueue 122 // and m_decoders only get modified on this queue. 123 void LibWebRTCCodecsProxy::decodeFrame(RTCDecoderIdentifier identifier, uint32_t timeStamp, const IPC::DataReference& data) WTF_IGNORES_THREAD_SAFETY_ANALYSIS 124 { 125 ASSERT(!isMainRunLoop()); 126 ASSERT(m_decoders.contains(identifier)); 139 127 auto decoder = m_decoders.get(identifier); 140 if (!decoder) { 141 ASSERT_IS_TESTING_IPC(); 142 return; 143 } 128 if (!decoder) 129 return; 130 144 131 if (webrtc::decodeFrame(decoder, timeStamp, data.data(), data.size())) 145 m_connection->send(Messages::LibWebRTCCodecs::FailedDecoding { identifier }, 0); 146 } 147 148 void LibWebRTCCodecsProxy::setFrameSize(RTCDecoderIdentifier identifier, uint16_t width, uint16_t height) 149 { 150 assertIsCurrent(workQueue()); 132 m_gpuConnectionToWebProcess.connection().send(Messages::LibWebRTCCodecs::FailedDecoding { identifier }, 0); 133 } 134 135 // For performance reasons, this function accesses m_decoders without locking. This is safe because this function runs on the libWebRTCCodecsQueue 136 // and m_decoders only get modified on this queue. 137 void LibWebRTCCodecsProxy::setFrameSize(RTCDecoderIdentifier identifier, uint16_t width, uint16_t height) WTF_IGNORES_THREAD_SAFETY_ANALYSIS 138 { 139 ASSERT(!isMainRunLoop()); 140 ASSERT(m_decoders.contains(identifier)); 151 141 auto decoder = m_decoders.get(identifier); 152 if (!decoder) { 153 ASSERT_IS_TESTING_IPC(); 154 return; 155 } 142 if (!decoder) 143 return; 144 156 145 webrtc::setDecoderFrameSize(decoder, width, height); 157 146 } … … 159 148 void LibWebRTCCodecsProxy::createEncoder(RTCEncoderIdentifier identifier, const String& formatName, const Vector<std::pair<String, String>>& parameters, bool useLowLatency) 160 149 { 161 assertIsCurrent(workQueue()); 150 ASSERT(!isMainRunLoop()); 151 Locker locker { m_lock }; 152 ASSERT(!m_encoders.contains(identifier)); 153 162 154 std::map<std::string, std::string> rtcParameters; 163 155 for (auto& parameter : parameters) 164 156 rtcParameters.emplace(parameter.first.utf8().data(), parameter.second.utf8().data()); 165 157 166 auto* encoder = webrtc::createLocalEncoder(webrtc::SdpVideoFormat { formatName.utf8().data(), rtcParameters }, makeBlockPtr([connection = m_connection, identifier](const uint8_t* buffer, size_t size, const webrtc::WebKitEncodedFrameInfo& info) {158 auto* encoder = webrtc::createLocalEncoder(webrtc::SdpVideoFormat { formatName.utf8().data(), rtcParameters }, makeBlockPtr([connection = Ref { m_gpuConnectionToWebProcess.connection() }, identifier](const uint8_t* buffer, size_t size, const webrtc::WebKitEncodedFrameInfo& info) { 167 159 connection->send(Messages::LibWebRTCCodecs::CompletedEncoding { identifier, IPC::DataReference { buffer, size }, info }, 0); 168 160 }).get()); 169 161 webrtc::setLocalEncoderLowLatency(encoder, useLowLatency); 170 auto result = m_encoders.add(identifier, Encoder { encoder, nullptr }); 171 ASSERT_UNUSED(result, result.isNewEntry || isTestingIPC()); 172 m_hasEncodersOrDecoders = true; 162 m_encoders.add(identifier, encoder); 173 163 } 174 164 175 165 void LibWebRTCCodecsProxy::releaseEncoder(RTCEncoderIdentifier identifier) 176 166 { 177 assertIsCurrent(workQueue()); 178 auto encoder = m_encoders.take(identifier); 179 if (!encoder.webrtcEncoder) { 180 ASSERT_IS_TESTING_IPC(); 181 return; 182 } 183 webrtc::releaseLocalEncoder(encoder.webrtcEncoder); 184 m_hasEncodersOrDecoders = !m_encoders.isEmpty() || !m_decoders.isEmpty(); 185 } 186 187 void LibWebRTCCodecsProxy::initializeEncoder(RTCEncoderIdentifier identifier, uint16_t width, uint16_t height, unsigned startBitrate, unsigned maxBitrate, unsigned minBitrate, uint32_t maxFramerate) 188 { 189 assertIsCurrent(workQueue()); 190 auto* encoder = findEncoder(identifier); 191 if (!encoder) { 192 ASSERT_IS_TESTING_IPC(); 193 return; 194 } 195 webrtc::initializeLocalEncoder(encoder->webrtcEncoder, width, height, startBitrate, maxBitrate, minBitrate, maxFramerate); 167 ASSERT(!isMainRunLoop()); 168 Locker locker { m_lock }; 169 ASSERT(m_encoders.contains(identifier)); 170 if (auto encoder = m_encoders.take(identifier)) 171 webrtc::releaseLocalEncoder(encoder); 172 } 173 174 // For performance reasons, this function accesses m_encoders without locking. This is safe because this function runs on the libWebRTCCodecsQueue 175 // and m_encoders only get modified on this queue. 176 void LibWebRTCCodecsProxy::initializeEncoder(RTCEncoderIdentifier identifier, uint16_t width, uint16_t height, unsigned startBitrate, unsigned maxBitrate, unsigned minBitrate, uint32_t maxFramerate) WTF_IGNORES_THREAD_SAFETY_ANALYSIS 177 { 178 ASSERT(!isMainRunLoop()); 179 ASSERT(m_encoders.contains(identifier)); 180 auto encoder = m_encoders.get(identifier); 181 if (!encoder) 182 return; 183 184 webrtc::initializeLocalEncoder(encoder, width, height, startBitrate, maxBitrate, minBitrate, maxFramerate); 196 185 } 197 186 … … 212 201 } 213 202 214 void LibWebRTCCodecsProxy::encodeFrame(RTCEncoderIdentifier identifier, WebCore::RemoteVideoSample&& sample, uint32_t timeStamp, bool shouldEncodeAsKeyFrame) 215 { 216 assertIsCurrent(workQueue()); 203 // For performance reasons, this function accesses m_encoders without locking. This is safe because this function runs on the libWebRTCCodecsQueue 204 // and m_encoders only get modified on this queue. 205 void LibWebRTCCodecsProxy::encodeFrame(RTCEncoderIdentifier identifier, WebCore::RemoteVideoSample&& sample, uint32_t timeStamp, bool shouldEncodeAsKeyFrame) WTF_IGNORES_THREAD_SAFETY_ANALYSIS 206 { 207 ASSERT(!isMainRunLoop()); 217 208 ASSERT(m_encoders.contains(identifier)); 218 209 auto encoder = m_encoders.get(identifier); … … 230 221 } 231 222 232 void LibWebRTCCodecsProxy::setEncodeRates(RTCEncoderIdentifier identifier, uint32_t bitRate, uint32_t frameRate) 233 { 234 assertIsCurrent(workQueue()); 223 // For performance reasons, this function accesses m_encoders without locking. This is safe because this function runs on the libWebRTCCodecsQueue 224 // and m_encoders only get modified on this queue. 225 void LibWebRTCCodecsProxy::setEncodeRates(RTCEncoderIdentifier identifier, uint32_t bitRate, uint32_t frameRate) WTF_IGNORES_THREAD_SAFETY_ANALYSIS 226 { 227 ASSERT(!isMainRunLoop()); 235 228 auto encoder = m_encoders.get(identifier); 236 229 if (!encoder) … … 242 235 bool LibWebRTCCodecsProxy::allowsExitUnderMemoryPressure() const 243 236 { 244 assertIsMainRunLoop(); 245 return !m_hasEncodersOrDecoders; 237 ASSERT(isMainRunLoop()); 238 Locker locker { m_lock }; 239 return m_encoders.isEmpty() && m_decoders.isEmpty(); 246 240 } 247 241
Note:
See TracChangeset
for help on using the changeset viewer.