Bug 222451 - REGRESSION(r273309) [GStreamer] webrtc/captureCanvas-webrtc-software-h264-baseline.html is flaky crashing inside libwebrtc
Summary: REGRESSION(r273309) [GStreamer] webrtc/captureCanvas-webrtc-software-h264-bas...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Philippe Normand
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-25 19:01 PST by Lauro Moura
Modified: 2021-03-05 02:21 PST (History)
14 users (show)

See Also:


Attachments
Debug bot crash log (145.55 KB, text/plain)
2021-02-25 19:01 PST, Lauro Moura
no flags Details
Patch (29.55 KB, patch)
2021-03-03 06:54 PST, Philippe Normand
no flags Details | Formatted Diff | Diff
Patch (29.61 KB, patch)
2021-03-04 08:33 PST, Philippe Normand
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lauro Moura 2021-02-25 19:01:55 PST
Created attachment 421595 [details]
Debug bot crash log

webrtc/captureCanvas-webrtc-software-h264-baseline.html

Some times the bots do not generate a correct trace, but I managed to reproduce the one attached locally. The failure is already tracked in bug216538.

Debug thread 1 trace:

Thread 1 (Thread 0x7f6f59efa700 (LWP 40993)):
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007f7050f2b855 in __GI_abort () at abort.c:79
#2  0x00007f7050f862f7 in __libc_message (action=action@entry=do_abort, fmt=fmt@entry=0x7f7051097e35 "%s\n") at ../sysdeps/posix/libc_fatal.c:155
#3  0x00007f7050f8d81c in malloc_printerr (str=str@entry=0x7f705109a1f0 "double free or corruption (!prev)") at malloc.c:5347
#4  0x00007f7050f8ee6c in _int_free (av=0x7f6e8c000020, p=0x7f6e8c098e80, have_lock=<optimized out>) at malloc.c:4317
#5  0x00007f706c49fe09 in webrtc::AlignedFree(void*) (mem_block=0x7f6e8c098ec0) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/memory/aligned_malloc.cc:95
#6  0x00007f706c262e9e in webrtc::AlignedFreeDeleter::operator()(void*) const (this=0x7f6e8c0073e0, ptr=0x7f6e8c098ec0) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/memory/aligned_malloc.h:52
#7  0x00007f706c262fe0 in std::unique_ptr<unsigned char, webrtc::AlignedFreeDeleter>::~unique_ptr() (this=0x7f6e8c0073e0, __in_chrg=<optimized out>) at /usr/include/c++/10.2.0/bits/unique_ptr.h:361
#8  0x00007f706c25f918 in webrtc::I420Buffer::~I420Buffer() (this=0x7f6e8c0073c0, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/api/video/i420_buffer.cc:59
#9  0x00007f706c263438 in rtc::RefCountedObject<webrtc::I420Buffer>::~RefCountedObject() (this=0x7f6e8c0073c0, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/ref_counted_object.h:55
#10 0x00007f706c263454 in rtc::RefCountedObject<webrtc::I420Buffer>::~RefCountedObject() (this=0x7f6e8c0073c0, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/ref_counted_object.h:55
#11 0x00007f706c26340d in rtc::RefCountedObject<webrtc::I420Buffer>::Release() const (this=0x7f6e8c0073c0) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/ref_counted_object.h:41
#12 0x00007f706b1a2351 in rtc::scoped_refptr<webrtc::VideoFrameBuffer>::~scoped_refptr() (this=0x7f6e8c059860, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/api/scoped_refptr.h:102
#13 0x00007f706c294056 in __gnu_cxx::new_allocator<std::_List_node<rtc::scoped_refptr<webrtc::VideoFrameBuffer> > >::destroy<rtc::scoped_refptr<webrtc::VideoFrameBuffer> >(rtc::scoped_refptr<webrtc::VideoFrameBuffer>*) (this=0x55dc66d33090, __p=0x7f6e8c059860) at /usr/include/c++/10.2.0/ext/new_allocator.h:156
#14 0x00007f706c293ebf in std::allocator_traits<std::allocator<std::_List_node<rtc::scoped_refptr<webrtc::VideoFrameBuffer> > > >::destroy<rtc::scoped_refptr<webrtc::VideoFrameBuffer> >(std::allocator<std::_List_node<rtc::scoped_refptr<webrtc::VideoFrameBuffer> > >&, rtc::scoped_refptr<webrtc::VideoFrameBuffer>*) (__a=..., __p=0x7f6e8c059860) at /usr/include/c++/10.2.0/bits/alloc_traits.h:531
#15 0x00007f706c293c94 in std::__cxx11::_List_base<rtc::scoped_refptr<webrtc::VideoFrameBuffer>, std::allocator<rtc::scoped_refptr<webrtc::VideoFrameBuffer> > >::_M_clear() (this=0x55dc66d33090) at /usr/include/c++/10.2.0/bits/list.tcc:77
#16 0x00007f706c29380c in std::__cxx11::_List_base<rtc::scoped_refptr<webrtc::VideoFrameBuffer>, std::allocator<rtc::scoped_refptr<webrtc::VideoFrameBuffer> > >::~_List_base() (this=0x55dc66d33090, __in_chrg=<optimized out>) at /usr/include/c++/10.2.0/bits/stl_list.h:499
#17 0x00007f706c29378e in std::__cxx11::list<rtc::scoped_refptr<webrtc::VideoFrameBuffer>, std::allocator<rtc::scoped_refptr<webrtc::VideoFrameBuffer> > >::~list() (this=0x55dc66d33090, __in_chrg=<optimized out>) at /usr/include/c++/10.2.0/bits/stl_list.h:827
#18 0x00007f706c292dd6 in webrtc::VideoFrameBufferPool::~VideoFrameBufferPool() (this=0x55dc66d33080, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/common_video/video_frame_buffer_pool.cc:52
#19 0x00007f706bdf98d4 in WebCore::GStreamerVideoFrameLibWebRTC::~GStreamerVideoFrameLibWebRTC() (this=0x55dc66d32fd0, __in_chrg=<optimized out>) at ../../Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.h:37
#20 0x00007f706bdf990c in WebCore::GStreamerVideoFrameLibWebRTC::~GStreamerVideoFrameLibWebRTC() (this=0x55dc66d32fd0, __in_chrg=<optimized out>) at ../../Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.h:37
#21 0x00007f706bdf9983 in rtc::RefCountedObject<webrtc::VideoFrameBuffer>::Release() const (this=0x55dc66d32fd0) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/ref_counted_object.h:41
#22 0x00007f706b1a2351 in rtc::scoped_refptr<webrtc::VideoFrameBuffer>::~scoped_refptr() (this=0x55dc5f041308, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/api/scoped_refptr.h:102
#23 0x00007f706c266132 in webrtc::VideoFrame::~VideoFrame() (this=0x55dc5f041300, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/api/video/video_frame.cc:282
#24 0x00007f706cc15352 in ~<lambda>(void) (this=0x55dc5f0412f8, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/video/video_stream_encoder.cc:976
#25 0x00007f706cc28bfc in webrtc::webrtc_new_closure_impl::ClosureTask<webrtc::VideoStreamEncoder::OnFrame(const webrtc::VideoFrame&)::<lambda()> >::~ClosureTask(void) (this=0x55dc5f0412f0, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/task_utils/to_queued_task.h:25
#26 0x00007f706cc28c24 in webrtc::webrtc_new_closure_impl::ClosureTask<webrtc::VideoStreamEncoder::OnFrame(const webrtc::VideoFrame&)::<lambda()> >::~ClosureTask(void) (this=0x55dc5f0412f0, __in_chrg=<optimized out>) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/task_utils/to_queued_task.h:25
#27 0x00007f706c4a7ffc in webrtc::(anonymous namespace)::TaskQueueStdlib::ProcessTasks() (this=0x7f6f0c10edc0) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/task_queue_stdlib.cc:242
#28 0x00007f706c4a7f2c in webrtc::(anonymous namespace)::TaskQueueStdlib::ThreadMain(void*) (context=0x7f6f0c10edc0) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/task_queue_stdlib.cc:226
#29 0x00007f70686ef5cf in rtc::PlatformThread::Run() (this=0x7f6f0c10eee8) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/platform_thread.cc:130
#30 0x00007f70686eed34 in rtc::PlatformThread::StartThread(void*) (param=0x7f6f0c10eee8) at ../../Source/ThirdParty/libwebrtc/Source/webrtc/rtc_base/platform_thread.cc:67
#31 0x00007f70515174d2 in start_thread (arg=<optimized out>) at pthread_create.c:477
#32 0x00007f7051007323 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

STDERR:

STDERR: (used_ids.h:55): Duplicate id found. Reassigning from 103 to 127
STDERR: (used_ids.h:55): Duplicate id found. Reassigning from 105 to 125
STDERR: (webrtc_video_engine.cc:3316): Absent receive stream; ignoring clearing encoded frame sink for ssrc 0
STDERR: (webrtc_video_engine.cc:2130): RTX SSRCs configured but there's no configured RTX payload type. Ignoring.
STDERR: (balanced_degradation_settings.cc:96): Unsupported size, value ignored.
STDERR: (balanced_degradation_settings.cc:96): Unsupported size, value ignored.
STDERR: (balanced_degradation_settings.cc:96): Unsupported size, value ignored.
STDERR: (video_send_stream_impl.cc:260): ERROR: Initial encoder max bitrate = -1 which is <= 0!
STDERR: (video_stream_encoder.cc:1461): Failed to encode frame. Error code: -1
STDERR: (generic_decoder.cc:94): Too many frames backed up in the decoder, dropping this one.
STDERR: (generic_decoder.cc:94): Too many frames backed up in the decoder, dropping this one.
STDERR: (generic_decoder.cc:94): Too many frames backed up in the decoder, dropping this one.
STDERR: (video_receive_stream2.cc:757): No decodable frame in 3000 ms, requesting keyframe.
STDERR: (rtcp_receiver.cc:315): std::optional<webrtc::TimeDelta> webrtc::RTCPReceiver::OnPeriodicRttUpdate(webrtc::Timestamp, bool): Timeout: No increase in RTCP RR extended highest sequence number.
STDERR: (video_stream_encoder.cc:1461): Failed to encode frame. Error code: -1
STDERR: double free or corruption (!prev)
STDERR: LEAK: 1 WebPageProxy
Comment 1 Philippe Normand 2021-02-26 08:24:26 PST
Investigation started...
Comment 2 Lauro Moura 2021-02-26 10:03:16 PST
Another test is timing out after the same revision:

webrtc/video-h264.html

History: https://results.webkit.org/?suite=layout-tests&test=webrtc%2Fvideo-h264.html
Comment 3 Philippe Normand 2021-03-03 06:54:39 PST
Created attachment 422074 [details]
Patch
Comment 4 Xabier Rodríguez Calvar 2021-03-04 07:21:23 PST
Comment on attachment 422074 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422074&action=review

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:34
> +        return framebuffer->takeSample();

WARN_UNUSED_RETURN GRefPtr<GstSample>&& GStreamerSampleFromLibWebRTCVideoFrame(const webrtc::VideoFrame& frame)

You seem to want a cascade of no copies and here there is one, explanation below.

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:38
> +    auto b = frame.video_frame_buffer()->ToI420();
> +    auto* i420Buffer = b.release();

You can do this in the same call, can't you?

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:46
> +    gsize offsets[3] = {

size_t?

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:49
> +        static_cast<gsize>(i420Buffer->StrideY() * height + i420Buffer->StrideU() * ((height +1) / 2))

height + 1

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:55
> +        const_cast<gpointer>(reinterpret_cast<const void*>(i420Buffer->DataY())),
> +        info.size, 0, info.size, i420Buffer, [](gpointer b) {

these lines can be one. And b -> buffer

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:56
> +            reinterpret_cast<webrtc::I420Buffer*>(b)->Release();

Rant: ain't it confusing they use Release here and then they wrap this in smart pointers where you have release() ?

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.h:46
> +    GRefPtr<GstSample> takeSample() { return WTFMove(m_sample); }

For the name and the code of what you're doing I assume the intention of this code is returning the sample we got without any copy and "thus" releasing what we have here. But it is not happening, there is an implicit copy here: what is happening here would be "equivalent" to, if I am not mistaken:

GRefPtr<GstSample> takeSample() {
    GRefPtr sample(WTFMove(m_sample)); // We create an object by moving what we have.
    return sample; // Copy elision does the magic and avoids the copy.
}

We're doing at least one copy of the pointer here, which I guess is not the intention and it works because of the "equivalence" I just explained.

For you to avoid this, you would need to do something like:

GRefPtr<GstSample>&& takeSample() { return WTFMove(m_sample); }

But this has a huge problem and it is that if you call this with the intention of just releasing the internal frame without assigning it anywhere like just:

framebuffer->takeFrame();

it will do absolutely nothing and your value will be still there after the call.

But worry not, we can has WARN_UNUSED_RETURN GRefPtr<GstSample>&& takeSample() { return WTFMove(m_sample); }
Comment 5 Philippe Normand 2021-03-04 08:33:03 PST
Created attachment 422232 [details]
Patch
Comment 6 EWS 2021-03-05 01:38:05 PST
Committed r273951: <https://commits.webkit.org/r273951>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422232 [details].
Comment 7 Radar WebKit Bug Importer 2021-03-05 01:39:15 PST
<rdar://problem/75085094>
Comment 8 Xabier Rodríguez Calvar 2021-03-05 01:45:27 PST
Comment on attachment 422232 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422232&action=review

> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:30
> +WARN_UNUSED_RETURN GRefPtr<GstSample> GStreamerSampleFromLibWebRTCVideoFrame(const webrtc::VideoFrame& frame)

You missed a && here, could you please fix it? Unreviewed is ok.
Comment 9 Philippe Normand 2021-03-05 01:54:58 PST
Oops! Follow-up in r273953 :)
Comment 10 Philippe Normand 2021-03-05 02:07:37 PST
and r273954
Comment 11 Xabier Rodríguez Calvar 2021-03-05 02:21:51 PST
Comment on attachment 422232 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=422232&action=review

>> Source/WebCore/platform/mediastream/gstreamer/GStreamerVideoFrameLibWebRTC.cpp:30
>> +WARN_UNUSED_RETURN GRefPtr<GstSample> GStreamerSampleFromLibWebRTCVideoFrame(const webrtc::VideoFrame& frame)
> 
> You missed a && here, could you please fix it? Unreviewed is ok.

Wow, no, I was very wrong. I was missing the rest of the function... We need to remove the &&. We can keep the warning.

Sorry for the noise and the error.