Closed Bug 1603349 Opened 2 years ago Closed 2 years ago

Crash in [@ @0x0 | Prolog_v1]

Categories

(Core :: WebRTC: Networking, defect, P2)

72 Branch
x86_64
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox-esr68 --- unaffected
firefox71 --- unaffected
firefox72 --- fixed
firefox73 --- fixed

People

(Reporter: philipp, Assigned: dminor)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug is for crash report bp-cde027a9-8a83-4727-9b91-79bd60191212.

Top 7 frames of crashing thread:

0  @0x0 
1 ws2_32.dll Prolog_v1 
2 xul.dll static union core::result::Result<mdns_service::ServiceControl, std::sync::mpsc::stream::Failure<mdns_service::ServiceControl>> std::sync::mpsc::stream::Packet<mdns_service::ServiceControl>::recv<mdns_service::ServiceControl> src/libstd/sync/mpsc/stream.rs:186
3 xul.dll static void mdns_service::{{impl}}::start::{{closure}} media/mtransport/mdns_service/src/lib.rs:283
4 xul.dll static void std::sys_common::backtrace::__rust_begin_short_backtrace<closure-0,  src/libstd/sys_common/backtrace.rs:77
5 xul.dll static void core::ops::function::FnOnce::call_once<closure-0,  src/libcore/ops/function.rs:235
6 xul.dll static void alloc::boxed::{{impl}}::call_once< src/liballoc/boxed.rs:787

this crash signature is starting to show up in the 72.0b cycle on windows 10 - it's mostly coming from users on the devedition channel though.

Assignee: nobody → dminor
Priority: -- → P2

There are known issues with recv_timeout panicing: https://github.com/rust-lang/rust/issues/39364, https://doc.rust-lang.org/std/sync/mpsc/struct.Receiver.html#known-issues that we might be hitting here, thanks Nico for finding those.

So we might have to rewrite this to avoid the blocking read by running the udp socket on its own thread, or maybe we can avoid this by restructuring the existing code. I'm not sure if it is sufficient to avoid this bug, but we could use a blocking read until we receive a message asking us to register or query a hostname as there is nothing for the UDP socket to do up until that point.

There are known bugs with recv_timeout which may explain the crashes we're
seeing in Bug 1603349. This patch switches to using try_recv which returns
immediately if no data is available. This thread already has timeouts set in
the UDP socket reads and writes, so the timeout with the receive channel
should not be necessary.

Pushed by dminor@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5b6ef512edd2
Use try_recv rather than recv_timeout in mdns_service; r=ng
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73

This might be safe enough to uplift?

Flags: needinfo?(dminor)

Comment on attachment 9116355 [details]
Bug 1603349 - Use try_recv rather than recv_timeout in mdns_service; r=ng!

Beta/Release Uplift Approval Request

  • User impact if declined: Potential crashes
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Internally, recv_timeout calls try_recv anyway, so this should hopefully skip the section of code where we're seeing the crash without changing other behaviour. This will change the timing in the receive loop a bit, but I don't expect this to cause problems, because I think there are relatively few control messages being received in comparison to the work done on the UDP socket. This feature is behind a pref, so if we see enough problems, we can pref this off for this release and try again in Firefox 73.
  • String changes made/needed: None
Flags: needinfo?(dminor)
Attachment #9116355 - Flags: approval-mozilla-beta?

Comment on attachment 9116355 [details]
Bug 1603349 - Use try_recv rather than recv_timeout in mdns_service; r=ng!

webrtc crash fix, approved for 72.0b9

Attachment #9116355 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.