Summary: | ThreadMessageReceiverRefCounted subclasses are accessed in non-thread-safe way | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kimmo Kinnunen <kkinnunen> | ||||||||||||
Component: | WebKit2 | Assignee: | Kimmo Kinnunen <kkinnunen> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | cdumez, eric.carlson, ews-watchlist, glenn, jer.noble, philipj, sergio, tsavell, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | Other | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=222056 | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 221560 | ||||||||||||||
Attachments: |
|
Description
Kimmo Kinnunen
2021-02-15 02:10:11 PST
Created attachment 420290 [details]
Patch
Created attachment 420291 [details]
Patch
Comment on attachment 420291 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420291&action=review r=me with nits. > Source/WebKit/GPUProcess/webrtc/RemoteAudioMediaStreamTrackRendererManager.cpp:46 > +void RemoteAudioMediaStreamTrackRendererManager::connectGPUProcess() I don't understand why this is named connectGPUProcess(). This code runs in the GPUProcess and if anything, it starts listening for IPC from the WebProcess. I think we should find a better name. Maybe startListeningForIPC()? > Source/WebKit/GPUProcess/webrtc/RemoteAudioMediaStreamTrackRendererManager.h:66 > + void disconnectGPUProcess(); What's this? Can't find the definition. > Source/WebKit/GPUProcess/webrtc/RemoteSampleBufferDisplayLayerManager.cpp:48 > +void RemoteSampleBufferDisplayLayerManager::connectGPUProcess() Same comment about naming. > Source/WebKit/NetworkProcess/webrtc/NetworkRTCProvider.cpp:98 > +void NetworkRTCProvider::connectWebProcess() Same comment about naming. > Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:171 > +void LibWebRTCCodecs::connectGPUProcess() Same comment about naming. > Source/WebKit/WebProcess/cocoa/RemoteCaptureSampleManager.cpp:52 > +void RemoteCaptureSampleManager::disconnectGPUProcess() Same comment about naming. Created attachment 420429 [details]
update names
Created attachment 420430 [details]
update names
commit-queue failed to commit attachment 420430 [details] to WebKit repository. To retry, please set cq+ flag again.
Committed r272905: <https://commits.webkit.org/r272905> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420430 [details]. Reverted r272905 for reason: caused TestWebKitAPI.WebKit2.CrashGPUProcessWhileCapturingAndCalling to timeout Committed r273029 (234230@main): <https://commits.webkit.org/234230@main> Created attachment 420813 [details]
Patch
Comment on attachment 420813 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=420813&action=review > Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:-179 > - m_connection->removeThreadMessageReceiver(Messages::LibWebRTCCodecsProxy::messageReceiverName()); This line was originally buggy.. > Source/WebKit/WebProcess/GPU/webrtc/LibWebRTCCodecs.cpp:471 > + m_connection->removeThreadMessageReceiver(Messages::LibWebRTCCodecs::messageReceiverName()); ..and I managed to put the above line here. Here in this patch this line is now correct and the test passes again Comment on attachment 420813 [details]
Patch
Taking the liberty to carry the r+ over from the last patch, as the only thing that changed was the ReceiverName change from Proxy message to plain message.
Committed r273074: <https://commits.webkit.org/r273074> All reviewed patches have been landed. Closing bug and clearing flags on attachment 420813 [details]. |