Closed
Bug 1387525
Opened 7 years ago
Closed 7 years ago
Crash in webrtc::NetEqImpl::InsertPacketInternal
Categories
(Core :: WebRTC, defect, P1)
Core
WebRTC
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | fixed |
firefox57 | --- | fixed |
People
(Reporter: philipp, Assigned: dminor)
References
(Blocks 1 open bug)
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
(deleted),
patch
|
jesup
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is
report bp-136e1645-191c-448e-a363-a9a1c0170804.
=============================================================
Crashing Thread (12), Name: Socket Thread
Frame Module Signature Source
0 xul.dll webrtc::NetEqImpl::InsertPacketInternal(webrtc::WebRtcRTPHeader const&, rtc::ArrayView<unsigned char const >, unsigned int) media/webrtc/trunk/webrtc/modules/audio_coding/neteq/neteq_impl.cc:694
1 xul.dll webrtc::NetEqImpl::InsertPacket(webrtc::WebRtcRTPHeader const&, rtc::ArrayView<unsigned char const >, unsigned int) media/webrtc/trunk/webrtc/modules/audio_coding/neteq/neteq_impl.cc:141
2 xul.dll webrtc::acm2::AcmReceiver::InsertPacket(webrtc::WebRtcRTPHeader const&, rtc::ArrayView<unsigned char const >) media/webrtc/trunk/webrtc/modules/audio_coding/acm2/acm_receiver.cc:107
3 xul.dll webrtc::`anonymous namespace'::AudioCodingModuleImpl::IncomingPacket media/webrtc/trunk/webrtc/modules/audio_coding/acm2/audio_coding_module.cc:1096
4 xul.dll webrtc::voe::Channel::OnReceivedPayloadData(unsigned char const*, unsigned __int64, webrtc::WebRtcRTPHeader const*) media/webrtc/trunk/webrtc/voice_engine/channel.cc:580
5 xul.dll webrtc::RTPReceiverAudio::ParseAudioCodecSpecific(webrtc::WebRtcRTPHeader*, unsigned char const*, unsigned __int64, webrtc::AudioPayload const&, bool) media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:304
6 xul.dll webrtc::RTPReceiverAudio::ParseRtpPacket(webrtc::WebRtcRTPHeader*, webrtc::PayloadUnion const&, bool, unsigned char const*, unsigned __int64, __int64, bool) media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_audio.cc:154
7 xul.dll webrtc::RtpReceiverImpl::IncomingRtpPacket(webrtc::RTPHeader const&, unsigned char const*, unsigned __int64, webrtc::PayloadUnion, bool) media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_impl.cc:186
8 xul.dll webrtc::voe::Channel::ReceivePacket(unsigned char const*, unsigned __int64, webrtc::RTPHeader const&, bool) media/webrtc/trunk/webrtc/voice_engine/channel.cc:1661
9 xul.dll webrtc::voe::Channel::ReceivedRTPPacket(unsigned char const*, unsigned __int64, webrtc::PacketTime const&) media/webrtc/trunk/webrtc/voice_engine/channel.cc:1643
10 xul.dll webrtc::VoENetworkImpl::ReceivedRTPPacket(int, void const*, unsigned __int64, webrtc::PacketTime const&) media/webrtc/trunk/webrtc/voice_engine/voe_network_impl.cc:87
11 xul.dll webrtc::VoENetworkImpl::ReceivedRTPPacket(int, void const*, unsigned __int64) media/webrtc/trunk/webrtc/voice_engine/voe_network_impl.cc:63
12 xul.dll mozilla::WebrtcAudioConduit::ReceivedRTPPacket(void const*, int, unsigned int) media/webrtc/signaling/src/media-conduit/AudioConduit.cpp:793
13 xul.dll mozilla::MediaPipeline::RtpPacketReceived(mozilla::TransportLayer*, unsigned char const*, unsigned __int64) media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:1124
14 xul.dll mozilla::MediaPipeline::PacketReceived(mozilla::TransportLayer*, unsigned char const*, unsigned __int64) media/webrtc/signaling/src/mediapipeline/MediaPipeline.cpp:1234
15 xul.dll sigslot::signal3<mozilla::TransportLayer*, unsigned char const*, unsigned __int64, sigslot::single_threaded>::operator()(mozilla::TransportLayer*, unsigned char const*, unsigned __int64) media/mtransport/sigslot.h:2486
16 xul.dll mozilla::TransportLayerIce::IcePacketReceived(mozilla::NrIceMediaStream*, int, unsigned char const*, int) media/mtransport/transportlayerice.cpp:224
17 xul.dll mozilla::NrIceCtx::msg_recvd(void*, nr_ice_peer_ctx_*, nr_ice_media_stream_*, int, unsigned char*, int) media/mtransport/nricectx.cpp:380
18 xul.dll nr_ice_peer_ctx_deliver_packet_maybe media/mtransport/third_party/nICEr/src/ice/ice_peer_ctx.c:837
19 xul.dll nr_ice_ctx_deliver_packet media/mtransport/third_party/nICEr/src/ice/ice_ctx.c:944
20 xul.dll nr_ice_socket_readable_cb media/mtransport/third_party/nICEr/src/ice/ice_socket.c:191
21 xul.dll mozilla::NrUdpSocketIpc::recv_callback_s(RefPtr<mozilla::nr_udp_message>) media/mtransport/nr_socket_prsock.cpp:1666
22 xul.dll mozilla::runnable_args_memfn<RefPtr<mozilla::NrUdpSocketIpc>, void ( mozilla::NrUdpSocketIpc::*)(RefPtr<mozilla::nr_udp_message>), RefPtr<mozilla::nr_udp_message> >::Run() media/mtransport/runnable_utils.h:172
23 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1446
24 xul.dll NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:480
25 xul.dll mozilla::net::nsSocketTransportService::Run() netwerk/base/nsSocketTransportService2.cpp:976
26 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1446
27 xul.dll NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:480
28 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:369
29 xul.dll MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:319
30 xul.dll MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:299
31 xul.dll nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:506
32 nss3.dll PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:397
33 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:95
34 ucrtbase.dll o__realloc_base
35 kernel32.dll BaseThreadInitThunk
36 ntdll.dll RtlUserThreadStart
this crash is cross platform and first appeared on nightly 56.0a1 build 20170617030206. some user comments indicate that they were using FireRTC when the crash happened.
Updated•7 years ago
|
Rank: 15
Priority: -- → P1
firefox crashed in firefox 56.0b2 (64-bit) beta once joining webrtc conference.
https://crash-stats.mozilla.com/report/index/06e5f867-0722-4c39-9cff-b2eeb0170815
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dminor
Status: NEW → ASSIGNED
Priority: P2 → P1
Assignee | ||
Comment 3•7 years ago
|
||
I had to disable one of the unit tests because it also assumes it will always get a valid decoder [1]. That would be a good place to start if we want to either start supporting more payload types or remove things from the list of types we claim to support.
[1] http://searchfox.org/mozilla-central/rev/e5b13e6224dbe3182050cf442608c4cb6a8c5c55/media/webrtc/trunk/gtest/moz.build#194
Attachment #8897395 -
Flags: review?(rjesup)
Comment 4•7 years ago
|
||
Comment on attachment 8897395 [details] [diff] [review]
Add check that we have a valid decoder
Review of attachment 8897395 [details] [diff] [review]:
-----------------------------------------------------------------
::: media/webrtc/trunk/webrtc/modules/audio_coding/neteq/neteq_impl.cc
@@ +691,5 @@
> AudioDecoder* decoder = decoder_database_->GetDecoder(main_payload_type);
> RTC_DCHECK(decoder); // Should always get a valid object, since we have
> // already checked that the payload types are known.
> + // The above assumption is not true for Mozilla webrtc.org builds.
> + if (!decoder) {
This will crash out because of the RTC_DCHECK in debug builds. We should probably either remove it and the comment (or modify the comment), or use a MOZILLA ifdef. In this case, I think relaxing the check to what we have is better than an ifdef. If so, remove the DCHECK, and modify the comments to describe how it works now.
Attachment #8897395 -
Flags: review?(rjesup) → review+
Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/89f3908b2c51
Fix crash in webrtc::NetEqImpl::InsertPacketInternal; r=jesup
Comment 6•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
(In reply to Wes Kocher (:KWierso) from comment #6)
> https://hg.mozilla.org/mozilla-central/rev/89f3908b2c51
will the modification be merged into firefox 56?
Comment 8•7 years ago
|
||
Please request Beta approval on this when you get a chance.
Flags: needinfo?(dminor)
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8897395 [details] [diff] [review]
Add check that we have a valid decoder
Approval Request Comment
[Feature/Bug causing the regression]:
This is in upstream webrtc.org code. They support codecs that we don't use and so make an assumption about always getting a valid decoder that isn't applicable for our build.
[User impact if declined]:
Crashes if an unsupported rtp type is used.
[Is this code covered by automated tests?]:
No, our tests currently only cover supported RTP payload types (or we would have seen this crash in automation.)
[Has the fix been verified in Nightly?]:
It will be in tonight's nightly.
[Needs manual test from QE? If yes, steps to reproduce]:
No.
[List of other uplifts needed for the feature/fix]:
None.
[Is the change risky?]:
No.
[Why is the change risky/not risky?]:
This only replaces a debug assert with a null check.
[String changes made/needed]:
None.
Flags: needinfo?(dminor)
Attachment #8897395 -
Flags: approval-mozilla-beta?
Comment 10•7 years ago
|
||
Comment on attachment 8897395 [details] [diff] [review]
Add check that we have a valid decoder
Fix a crash related to webrtc. Beta56+.
Attachment #8897395 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 11•7 years ago
|
||
bugherder uplift |
Assignee | ||
Updated•6 years ago
|
Blocks: webrtc_upstream_bugs
Assignee | ||
Comment 12•6 years ago
|
||
We should upstream this fix.
You need to log in
before you can comment on or make changes to this bug.
Description
•