1702529 - Crash in [@ mozilla::net::Http3Session::ProcessInput]

Status: RESOLVED FIXED

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

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

4 crashes on 2+ devices, all on Windows 10

Crash report: https://crash-stats.mozilla.org/report/index/24b0dd56-4154-4319-a431-d090f0210331

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (NS_SUCCEEDED(rv))

Top 10 frames of crashing thread:

0 xul.dll mozilla::net::Http3Session::ProcessInput netwerk/protocol/http/Http3Session.cpp:325
1 xul.dll mozilla::net::Http3Session::RecvData netwerk/protocol/http/Http3Session.cpp:1149
2 xul.dll mozilla::net::HttpConnectionUDP::RecvData netwerk/protocol/http/HttpConnectionUDP.cpp:544
3 xul.dll mozilla::net::HttpConnectionUDP::OnPacketReceived netwerk/protocol/http/HttpConnectionUDP.cpp:646
4 xul.dll mozilla::net::nsSocketTransportService::Run netwerk/base/nsSocketTransportService2.cpp:1160
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1145
6 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:302
7 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:328
8 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:310
9 xul.dll static nsThread::ThreadFunc xpcom/threads/nsThread.cpp:391

Comment 1

[Dragana Damjanovic [:dragana]]

This is failing because rust parser of an IP address in form of a string fails.
The protocol will handle just fine some of the packets being dropet for his reason.
I will leave this as a diagnostic assert, and I will lot to change c++-rust glue code a bit to mitigate this problem if possible.

Comment 2

[Valentin Gosu [:valentin] (he/him)]

It seems the result of AddrToString when the assertion happens here is ?:443 - which obviously doesn't parse as an IP addr.
Now I'm trying to figure out where the ? is coming from 🙂

Comment 3

[Valentin Gosu [:valentin] (he/him)]

From what I can tell the ? is coming from getnameinfo via inet_ntop_internal.

From what I can tell, because we pass NI_NUMERICHOST, this should never happen.
Maybe a bug in the getnameinfo implementation on windows?

In any case, the steps here seem to be:

1. Pass NetAddr/SockAddr to and from rust, instead of parsing the IP on every call.
2. Use a native serialization of NetAddr to avoid said bug (if it exists) in a follow-up bug.

Comment 4

[Valentin Gosu [:valentin] (he/him)]

Attachment: Bug 1702529 - Pass NetAddr pointers to rust instead of a serialization r=#necko

It seems that the serialization generated by inet_ntop_internal via
getnameinfo is sometimes wrong, returning ? instead of serializing the
IP we pass in.
It's also inefficient to keep passing the serialization to and from rust
code - instead it's much easier to just pass a pointer to the NetAddr
union and build a proper SocketAddr instance on the rust side from
from the bytes instead of parsing the serialization

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba94ff2a7696
Pass NetAddr pointers to rust instead of a serialization r=necko-reviewers,dragana

Comment 6

[Cristian Tuns]

Backed out for causing build bustages.

• Backout link
• Push with failures
• Failure Log

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/241563557fd4
Pass NetAddr pointers to rust instead of a serialization r=necko-reviewers,dragana

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/241563557fd4