1776143 - Crash in [@ RtlpWaitOnCriticalSection | RtlpEnterCriticalSectionContended | RtlEnterCriticalSection | sctp_inpcb_free | sctp_close]

Status: NEW

(Core :: WebRTC: Networking, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/054f6c00-b865-4941-b5ba-21ec80220623

Reason: EXCEPTION_ACCESS_VIOLATION_WRITE

Top 10 frames of crashing thread:

0 ntdll.dll RtlpWaitOnCriticalSection 
1 ntdll.dll RtlpEnterCriticalSectionContended 
2 ntdll.dll RtlEnterCriticalSection 
3 xul.dll sctp_inpcb_free netwerk/sctp/src/netinet/sctp_pcb.c:3857
4 xul.dll sctp_close netwerk/sctp/src/netinet/sctp_usrreq.c:842
5 xul.dll sofree netwerk/sctp/src/user_socket.c:287
6 xul.dll mozilla::DataChannelConnection::DestroyOnSTS netwerk/sctp/datachannel/DataChannel.cpp:399
7 xul.dll mozilla::detail::runnable_args_base<mozilla::detail::NoResult>::Run dom/media/webrtc/transport/runnable_utils.h:41
8 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:465
9 xul.dll mozilla::net::nsSocketTransportService::Run netwerk/base/nsSocketTransportService2.cpp:1202

It appears we're trying to lock a mutex that has been set to NULL. The crash seems to happen only on Windows but bug 1775214 points to the same issue on Linux. This does not appear to be a new bug but it was recently detected by clouseau due to a visible spike.

Comment 1

[Gabriele Svelto [:gsvelto]]

Alright, now that I can see the whole graph this looks like a regression introduced in version 100. The volume here is non-trivial.

Comment 2

[Gabriele Svelto [:gsvelto]]

There's several more signatures for this, ouch. At least one of them is on Android so this is indeed a problem that affects all platforms albeit with different signatures.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Quite a few crashes on related signatures; there's some type of race here, though it doesn't seem security-sensitive

Comment 4

[Michael Tüxen]

You might want to update to the current version. There are still a couple of known issues, but more on the receive path, not sending or closing.

Comment 5

[Jim Mathies [:jimm]]

shifting deps here slightly.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

For the record, updating libusrsctp to the latest version didn't seem to affect this. Anything we can do to move this forward?

Comment 7

[BugBot [:suhaib / :marco/ :calixte]]

The bug is linked to a topcrash signature, which matches the following criterion:

• Top 10 content process crashes on beta

For more information, please visit BugBot documentation.

Comment 8

[Michael Tüxen]

Is there any more information than the stack traces?

Comment 9

[Jim Mathies [:jimm]]

Jesup, do you have any insight here? We're kind of stalled out.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

No. I'll try to look more deeply. Michael, since updating didn't help, can you look to see what possible paths might lead to this?

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

Based on the topcrash criteria, the crash signatures linked to this bug are not in the topcrash signatures anymore.

For more information, please visit BugBot documentation.

Comment 13

[Intermittent Failures Robot]

1 failures in 3076 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-beta: 1

Platform and build breakdown:

• windows11-64-2009-asan-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1776143&startday=2023-08-14&endday=2023-08-20&tree=all

Comment 14

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.

Comment 15

[Intermittent Failures Robot]

1 failures in 4034 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-central: 1

Platform and build breakdown:

• windows11-64-2009-asan-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1776143&startday=2024-01-22&endday=2024-01-28&tree=all

Comment 16

[Gabriele Svelto [:gsvelto]]

Cleaning up the signatures after bug 1895527, adding a bunch of Android ones. The crash appear to be still valid and has significant volume, can we get someone to look into it?

Comment 17

[Intermittent Failures Robot]

1 failures in 3995 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• autoland: 1

Platform and build breakdown:

• windows11-64-2009-asan-qr: 1
  • opt: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=1776143&startday=2024-05-20&endday=2024-05-26&tree=all

Comment 18

[Chris Peterson [:cpeterson]]

Added a few more top Android signatures

Comment 19

[Byron Campen [:bwc]]

This might be related:

https://github.com/sctplab/usrsctp/issues/713

Comment 20

[Byron Campen [:bwc]]

Ok, not related, since we always build with that check true. The init and finish functions are pretty complicated, and might have holes when parts of init fail. Looking into it...

Comment 21

[Byron Campen [:bwc]]

I've looked at this for a while, and while there are flaws in the init/deinit functions, I'm not seeing one that would cause this specific problem on Windows. I do see that there is no error-checking for the initialization of this mutex/critical section, but the documentation for InitializeCriticalSection says that it is infallible on modern versions of windows, and this happens pretty much only on Windows and Android. It might be that this documentation is wrong or misleading.

I did notice a couple of flaws in our code that muddy the waters somewhat, so I think I'll fix them and hope that we get some more clarity on what is going on here.

Comment 22

[Byron Campen [:bwc]]

Attachment: Bug 1776143: Fix leak on init failure, make DataChannelRegistry non-refcounted, and add some assertions. r?ng

Comment 23

[Byron Campen [:bwc]]

https://treeherder.mozilla.org/jobs?repo=try&revision=343e4682f30e13133ed1f0c453b335505999e799
https://treeherder.mozilla.org/jobs?repo=try&revision=9dc33973ec40da84059f681dc5de6e8363ad0e06

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/23320214ae00
Fix leak on init failure, make DataChannelRegistry non-refcounted, and add some assertions. r=ng

Comment 25

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/23320214ae00