1852801 - startup Crash in [@ mozilla::widget::WinEventHub::GetWnd]

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

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

Not a new crash signature. Approximately 90 crashes for Firefox 116.0.x, all on Windows.

Crash report: https://crash-stats.mozilla.org/report/index/7dc37843-ce1a-4cd2-aaa7-f6be20230911

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0  xul.dll  mozilla::widget::WinEventHub::GetWnd  widget/windows/WinEventObserver.h:45
0  xul.dll  mozilla::widget::SessionChangeObserver::SessionChangeObserver  widget/windows/WinEventObserver.cpp:181
1  xul.dll  mozilla::widget::SessionChangeObserver::Create  widget/windows/WinEventObserver.cpp:171
2  xul.dll  mozilla::widget::WinWindowOcclusionTracker::WinWindowOcclusionTracker  widget/windows/WinWindowOcclusionTracker.cpp:536
3  xul.dll  mozilla::widget::WinWindowOcclusionTracker::Ensure  widget/windows/WinWindowOcclusionTracker.cpp:365
4  xul.dll  gfxPlatform::InitLayersIPC  gfx/thebes/gfxPlatform.cpp:1308
4  xul.dll  gfxPlatform::Init  gfx/thebes/gfxPlatform.cpp:973
5  xul.dll  gfxPlatform::GetPlatform  gfx/thebes/gfxPlatform.cpp:463
6  xul.dll  gfxPlatform::InitializeCMS  gfx/thebes/gfxPlatform.cpp:2107
7  xul.dll  gfxPlatform::EnsureCMSInitialized  gfx/thebes/gfxPlatform.h:968

Comment 1

[Brad Werth [:bradwerth] (out until Aug 17)]

Interesting. It seems that [this call to WinEventHub::Ensure() is failing to generate the singleton in WinEventHub::Initialize(). Indeed, the crash report notes a "Last Error Value ERROR_CANNOT_FIND_WND_CLASS" which seems like the sort of error that might be thrown in that case.

But what to do about it? I'll make WinEventHub::Ensure() return a bool, but all that would do is keep mSessionChangeObserver null and turn off occlusion tracking. That will fix the crash, but we'd have no occlusion tracking which would silently lead to other problems. I'll create a patch that does that and if the reviewers see a better solution, I'll refine it.

Comment 2

[Brad Werth [:bradwerth] (out until Aug 17)]

Attachment: Bug 1852801: Allow WinWindowOcclusionTracker members to fail initialization without crashing.

This patch also expands the use of EnsureDisplayStatusObserver and
EnsureSessionChangeObserver, using them during the WinWindowOcclusionTracker
constructor, and also in WinWindowOcclusionTracker::Ensure. This gives another
opportunity for these observers to be created when
WinUtils::EnableWindowOcclusion is called.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59ff0be2d94f
Allow WinWindowOcclusionTracker members to fail initialization without crashing. r=rkraesig,win-reviewers

Comment 4

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/59ff0be2d94f

Comment 5

[BugBot [:suhaib / :marco]]

The patch landed in nightly and beta is affected.
:bradwerth, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox118 to wontfix.

For more information, please visit BugBot documentation.

Comment 6

[Brad Werth [:bradwerth] (out until Aug 17)]

Comment on attachment 9352773 [details]
Bug 1852801: Allow WinWindowOcclusionTracker members to fail initialization without crashing.

Beta/Release Uplift Approval Request

• User impact if declined: Occasional startup crashes in Windows in unusual situations.
• Is this code covered by automated tests?: No
• 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): This just null-checks a few areas of code. What would have triggered a crash will now instead trigger some confusion about whether or not the window is fully occluded, which could lead to increased power consumption for fully-occluded windows.
• String changes made/needed:
• Is Android affected?: No

Comment 7

[Pascal Chevrel (relman team) -> :pascalc|PTO until Sept 2]

Comment on attachment 9352773 [details]
Bug 1852801: Allow WinWindowOcclusionTracker members to fail initialization without crashing.

Approved for landing on mozilla-beta before the merge, will be in the 118.0 release candidate, thanks.

Comment 8

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-beta/rev/639ac87aea17

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9352773 [details]
Bug 1852801: Allow WinWindowOcclusionTracker members to fail initialization without crashing.

Approved for 115.3esr

Comment 10

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/3dcf3dc332c5