Summary: | Safari WebKitWebRTCAudioModule crash during <video> tag update when audio track present in MediaStream | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | pmikolajczak | ||||||||
Component: | WebRTC | Assignee: | youenn fablet <youennf> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, darin, eric.carlson, jer.noble, jonlee, webkit-bug-importer, youennf | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | Safari 11 | ||||||||||
Hardware: | Mac | ||||||||||
OS: | macOS 10.12 | ||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=183988 | ||||||||||
Attachments: |
|
Thanks for the report pmikolajczak. In the crash logs, AudioTrackPrivateMediaStreamCocoa is destroyed in a background thread since AudioTrackPrivateMediaStreamCocoa::audioSamplesAvailable is taking a ref from a background thread. Removing the tracks might remove all other refs of the AudioTrackPrivateMediaStreamCocoa. Created attachment 335633 [details]
Patch
Comment on attachment 335633 [details] Patch Rejecting attachment 335633 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 335633, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: (offset 1 line). 1 out of 3 hunks FAILED -- saving rejects to file Source/WebCore/platform/mediastream/mac/AudioTrackPrivateMediaStreamCocoa.cpp.rej patching file LayoutTests/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file LayoutTests/webrtc/video-update-often-expected.txt patching file LayoutTests/webrtc/video-update-often.html Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Eric Carlson']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: http://webkit-queues.webkit.org/results/6927626 Created attachment 336371 [details]
Patch
Comment on attachment 336371 [details] Patch Clearing flags on attachment: 336371 Committed r229898: <https://trac.webkit.org/changeset/229898> All reviewed patches have been landed. Closing bug. This seems like a fragile fix. If we ever add a deref on a background thread by accident, the bug will be reintroduced. Is there some way we can fix it in a way that does not require us to be careful about every deref? (In reply to Darin Adler from comment #8) > This seems like a fragile fix. If we ever add a deref on a background thread > by accident, the bug will be reintroduced. Is there some way we can fix it > in a way that does not require us to be careful about every deref? Given the hierarchy of this class, doing some cleaning of the members in the destructor does not seem great. One approach would be to introduce a ThreadSafeRefCounted that would always destroy itself in the MainThread, something like: class TrackPrivateBase : public ThreadSafeRefCounted<TrackPrivateBase, Destruction::InMainThread> Filed https://bugs.webkit.org/show_bug.cgi?id=183988 to improve on this. |
Created attachment 330233 [details] Four crash logs I use RTCPeerConnection to stream audio and video from Chrome (63.0.3239.84) to Safari (Version 11.0.2 (12604.4.7.1.4)). Connection is established, audio and video are present on both sides. Then after a while in Safari, in my HTML app I update <video> tag that contains remote MediaStream (in srcObject) received from Chrome. By update I mean that I add and remove that video tag from HTML few times in row. It is done by some template rendering engine. During that update process Safari crashes (not always, but eventually will after several tries). Other few things: - video tag has autoplay, muted, playsinline attributes and couple of css rules. - updating video with only MediaStreamTrack kind "video" won't trigger crash. Audio track has to be present. - I am able to reproduce it locally by calling several times something like setInterval(updateVideo, 0), where updateVideo is: $parent.removeChild($video); $parent.appendChild($video); $video.srcObject = stream; Note: This is test case only, in my app updateVideo is not called that many times. - crash logs in attachments. One of them is from High Sierra, Safari Technology Preview. Other from Sierra 10.12.6. This can be found in logs: Crashed Thread: WebKitWebRTCAudioModule, abort() called, *** error for object 0x7fd6e61ac4d0: pointer being freed was not allocated