Closed Bug 1387525 Opened 7 years ago Closed 7 years ago

Crash in webrtc::NetEqImpl::InsertPacketInternal

Categories

(Core :: WebRTC, defect, P1)

defect

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)

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.
Rank: 15
Priority: -- → P1
unassigned -> p2
Priority: P1 → P2
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: nobody → dminor
Status: NEW → ASSIGNED
Priority: P2 → P1
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 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
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?
Please request Beta approval on this when you get a chance.
Flags: needinfo?(dminor)
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 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+
We should upstream this fix.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: