1851991 - Startup Crash in [@ mozilla::gfx::GPUProcessManager::SetAppInForeground]

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

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

This crash signature existed before but often only one instance reported crashes. For the beta versions of Firefox 118, there are 3-4 installs which reported this. 58 crashes on Windows 10 and 4 on Windows 11 over the past 3 months.

Crash report: https://crash-stats.mozilla.org/report/index/bc2e8578-4813-4488-951b-8bc8c0230906

Reason: EXCEPTION_ACCESS_VIOLATION_READ

Top 10 frames of crashing thread:

0  xul.dll  mozilla::gfx::GPUProcessManager::SetAppInForeground  gfx/ipc/GPUProcessManager.cpp:1516
0  xul.dll  nsWindow::ProcessMessageInternal  widget/windows/nsWindow.cpp:5895
1  xul.dll  nsWindow::ProcessMessage  widget/windows/nsWindow.cpp:4890
1  xul.dll  nsWindow::WindowProcInternal  widget/windows/nsWindow.cpp:4842
2  xul.dll  CallWindowProcCrashProtected  xpcom/base/nsCrashOnException.cpp:27
2  xul.dll  nsWindow::WindowProc  widget/windows/nsWindow.cpp:4795
3  user32.dll  UserCallWinProcCheckWow  
4  user32.dll  DispatchClientMessage  
5  user32.dll  _fnDWORD  
6  ntdll.dll  KiUserCallbackDispatch  

Further down the stack:

 36 	xul.dll	mozilla::widget::TSFTextStore::Initialize()	widget/windows/TSFTextStore.cpp:6795
37 	xul.dll	mozilla::widget::IMEHandler::Initialize()	widget/windows/WinIMEHandler.cpp:68
38 	xul.dll	mozilla::widget::IMEHandler::InitInputContext(nsWindow*, mozilla::widget::InputContext&)	widget/windows/WinIMEHandler.cpp:490
38 	xul.dll	nsWindow::Create(nsIWidget*, void*, mozilla::gfx::IntRectTyped<mozilla::LayoutDevicePixel> const&, mozilla::widget::InitData*)	widget/windows/nsWindow.cpp:1203
39 	xul.dll	nsIWidget::Create(nsIWidget*, void*, mozilla::gfx::IntRectTyped<mozilla::DesktopPixel> const&, mozilla::widget::InitData*)	widget/nsIWidget.h:463
40 	xul.dll	mozilla::AppWindow::Initialize(nsIAppWindow*, nsIAppWindow*, int, int, bool, mozilla::widget::InitData&)	xpfe/appshell/AppWindow.cpp:212
40 	xul.dll	nsAppShellService::JustCreateTopWindow(nsIAppWindow*, nsIURI*, unsigned int, int, int, bool, mozilla::AppWindow**)	xpfe/appshell/nsAppShellService.cpp:671
41 	xul.dll	nsAppShellService::CreateTopLevelWindow(nsIAppWindow*, nsIURI*, unsigned int, int, int, nsIAppWindow**)	xpfe/appshell/nsAppShellService.cpp:178
42 	xul.dll	nsAppStartup::CreateChromeWindow(nsIWebBrowserChrome*, unsigned int, nsIOpenWindowInfo*, bool*, nsIWebBrowserChrome**)	toolkit/components/startup/nsAppStartup.cpp:756 

Could this be related to the onscreen keyboard shown from the start with the use of initialization code since bug 1846476 landed?

Comment 1

[Masatoshi Kimura [:emk]]

Why needinfo'ed from me? I did neither write nor review the patch in bug 1846476. Bug 1846476 should not change the behavior on Win10+, but redirecting to Greg for verification.

Comment 2

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

Sorry, the needinfo request was supposed to be for Masayuki.

Comment 3

[Gregory Pappas [:gregp]]

Yeah, I don't think bug 1846476 has anything to do with this crash. It's a very simple patch that shouldn't have changed behavior on Win10+

Comment 4

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

The bug is marked as tracked for firefox118 (beta). We have limited time to fix this, the soft freeze is in 13 days. However, the bug still isn't assigned.

:gcp, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(away: 6/25-6/28)]

I think that the GPUProcessManager::Get() caller in widget/windows should ensure the instance or put a user/app message into the queue if it's not initialized yet.

Florian Queze, you added the call, could you take a look?

Comment 6

[Florian Quèze [:florian]]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #5)

Florian Queze, you added the call, could you take a look?

Not in the near future (I'm on parental leave).

Comment 7

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

This is a reminder regarding comment #4!

The bug is marked as tracked for firefox118 (beta). We have limited time to fix this, the soft freeze is in 7 days. However, the bug still isn't assigned.

Comment 8

[Florian Quèze [:florian]]

Maybe Bas could help? He was also involved in bug 1671490.

Comment 9

[Bas Schouten (:bas.schouten)]

Hmm.. Could this just be happening because there is no GPU process in some cases? In any case. Writing a patch.

Comment 10

[Bas Schouten (:bas.schouten)]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #5)

I think that the GPUProcessManager::Get() caller in widget/windows should ensure the instance or put a user/app message into the queue if it's not initialized yet.

Florian Queze, you added the call, could you take a look?

Hmm, so it does look like we always create the singleton regardless of whether there is a GPU process:

https://searchfox.org/mozilla-central/source/gfx/thebes/gfxPlatform.cpp#818

It's not great that this gets run before we've even run gfxPlatform::Init. I'm not sure I understand why.

Comment 11

[Bas Schouten (:bas.schouten)]

Attachment: Bug 1851991: Do not try to raise the GPU process priority when it hasn't been initialized yet. r=jrmuizel

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5fef89467c73
Do not try to raise the GPU process priority when it hasn't been initialized yet. r=jrmuizel

Comment 13

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5fef89467c73

Comment 14

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

The patch landed in nightly and beta is affected.
:bas.schouten, 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 15

[Bas Schouten (:bas.schouten)]

Comment on attachment 9353253 [details]
Bug 1851991: Do not try to raise the GPU process priority when it hasn't been initialized yet. r=jrmuizel

Beta/Release Uplift Approval Request

• User impact if declined: A small number of users will crash.
• 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): Simple null pointer check, if that check would fail right now, we would crash.
• String changes made/needed: None.
• Is Android affected?: No

Comment 16

[Pascal Chevrel:pascalc]

Keeping it on the radar for the planned dot release.

Comment 17

[Yannis Juglaret [:yannis]]

(In reply to Bas Schouten (:bas.schouten) from comment #10)

It's not great that this gets run before we've even run gfxPlatform::Init. I'm not sure I understand why.

If that helps, there are more details in bug 1856637 about how we reach our WindowProc here. As far as I can tell, the path is the following:

• XREMain::XRE_mainRun calls NS_CreateServicesFromCategory for app-startup;
• this makes nsBrowserGlue observe app-startup, which calls _earlyBlankFirstPaint and thus Services.ww.openWindow;
• from nsWindowWatcher::OpenWindow we reach mozilla::widget::TSFTextStore::Initialize, which calls a Microsoft method CThreadInputMgr::Activate;
• this Microsoft method internally calls PeekMessage and flushes the message queue for our window.

Comment 18

[Pascal Chevrel:pascalc]

Comment on attachment 9353253 [details]
Bug 1851991: Do not try to raise the GPU process priority when it hasn't been initialized yet. r=jrmuizel

Approved for our next 118 dot release, thanks.

Comment 19

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-release/rev/9cac3e0a2acd

Comment 20

[Pascal Chevrel:pascalc]

https://hg.mozilla.org/releases/mozilla-release/rev/9cac3e0a2acd

Comment 21

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/e90e96cadfa2

Comment 22

[Donal Meehan [:dmeehan]]

Comment on attachment 9353253 [details]
Bug 1851991: Do not try to raise the GPU process priority when it hasn't been initialized yet. r=jrmuizel

Approved for 115.4esr.