1747939 - Crash in [@ nsThread::ThreadFunc] in DNS Resolver with Firefox 96+

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

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

There are between 10 and 60 affected installations per Firefox 96 Beta version, and ~80 Nightly installations. Half the crashes have been observed after >1h uptime, a quarter on startup. The release assert got added in bug 1741377.

Jens: It's the 30th most frequent crash for betas in the last 4 weeks. Does any action need to be taken here?

Crash report: https://crash-stats.mozilla.org/report/index/0115b35a-5d79-4c4d-b859-7c5780211229

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(((bool)(__builtin_expect(!!(!NS_FAILED_impl(dispatch_ack_rv)), 1))))

Top 7 frames of crashing thread:

0 xul.dll static nsThread::ThreadFunc xpcom/threads/nsThread.cpp:443
1 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:399
2 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:139
3 ucrtbase.dll thread_start<unsigned int , 1> 
4 kernel32.dll BaseThreadInitThunk 
5 mozglue.dll patched_BaseThreadInitThunk toolkit/xre/dllservices/mozglue/WindowsDllBlocklist.cpp:574
6 ntdll.dll RtlUserThreadStart 

Comment 1

[Ryan VanderMeulen [:RyanVM]]

This looks like a pretty high volume crash. Tracking for 96/97.

Comment 2

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1747939 - Relax dispatch_ack assertion to handle ShutdownWithTimeout, r=jstutte

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 9257564 [details]
Bug 1747939 - Relax dispatch_ack assertion to handle ShutdownWithTimeout, r=jstutte

Beta/Release Uplift Approval Request

• User impact if declined: Shutdown assertion crashes caused by bad interaction between DNS resolver and XPCOM thread shutdown
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• 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): Any problems in this code already existed before the change which introduced this crash, the crash was only introduced as an assertion check. This change removes the assertion in the problematic case and should be low risk.
• String changes made/needed: none

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b18355c5bbd8
Relax dispatch_ack assertion to handle ShutdownWithTimeout, r=jstutte

Comment 5

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/b18355c5bbd8

Comment 6

[Dianna Smith [:diannaS]]

Comment on attachment 9257564 [details]
Bug 1747939 - Relax dispatch_ack assertion to handle ShutdownWithTimeout, r=jstutte

Approved for 96.0rc2

Comment 7

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-release/rev/1f7607f6bc24