Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PGO Training runs fail, TerminalApp.dll fails to load #10265

Closed
miniksa opened this issue May 28, 2021 · 7 comments · Fixed by #10370
Closed

PGO Training runs fail, TerminalApp.dll fails to load #10265

miniksa opened this issue May 28, 2021 · 7 comments · Fixed by #10370
Assignees
@miniksa
Labels
Area-Build Issues pertaining to the build system, CI, infrastructure, meta Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Meta The product is the management of the products. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@miniksa
Copy link
Member

miniksa commented May 28, 2021 

00 00000048`ce0fdb70 00007ffa`99c9d2df     ucrtbase!abort+0x4e [minkernel\crts\ucrt\src\appcrt\startup\abort.cpp @ 77] 
01 00000048`ce0fdba0 00007ffa`96441abf     ucrtbase!terminate+0x1f [minkernel\crts\ucrt\src\appcrt\misc\terminate.cpp @ 58] 
02 00000048`ce0fdbd0 00007ffa`9644232b     VCRUNTIME140_1!FindHandler<__FrameHandler4>+0x46f [d:\agent\_work\1\s\src\vctools\crt\vcruntime\src\eh\frame.cpp @ 682] 
03 00000048`ce0fdda0 00007ffa`964440e9     VCRUNTIME140_1!__InternalCxxFrameHandler<__FrameHandler4>+0x267 [d:\agent\_work\1\s\src\vctools\crt\vcruntime\src\eh\frame.cpp @ 352] 
04 00000048`ce0fde40 00007ff6`424b3918     VCRUNTIME140_1!__CxxFrameHandler4+0xa9 [d:\agent\_work\1\s\src\vctools\crt\vcruntime\src\eh\risctrnsctrl.cpp @ 290] 
05 00000048`ce0fdeb0 00007ffa`9cd411cf     WindowsTerminal!__GSHandlerCheck_EH4+0x64 [D:\agent\_work\10\s\src\vctools\crt\vcstartup\src\gs\amd64\gshandlereh4.cpp @ 86] 
06 00000048`ce0fdee0 00007ffa`9cd0a209     ntdll!RtlpExecuteHandlerForException+0xf [minkernel\ntos\rtl\amd64\xcptmisc.asm @ 132] 
07 00000048`ce0fdf10 00007ffa`9cd09fc3     ntdll!RtlDispatchException+0x219 [minkernel\ntos\rtl\amd64\exdsptch.c @ 693] 
08 00000048`ce0fe620 00007ffa`9a893b29     ntdll!RtlRaiseException+0x153 [minkernel\ntos\rtl\amd64\raise.c @ 182] 
09 00000048`ce0ff4d0 00007ffa`96426480     KERNELBASE!RaiseException+0x69 [minkernel\kernelbase\xcpt.c @ 937] 
0a 00000048`ce0ff5b0 00007ff6`4249799a     VCRUNTIME140!_CxxThrowException+0x90 [D:\agent\_work\10\s\src\vctools\crt\vcruntime\src\eh\throw.cpp @ 75] 
0b 00000048`ce0ff610 00007ff6`4248471b     WindowsTerminal!winrt::throw_hresult+0x3ba [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\base.h @ 4892] 
0c 00000048`ce0ff670 00007ff6`42484986     WindowsTerminal!winrt::check_hresult+0x2b [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\base.h @ 4962] 
0d 00000048`ce0ff6b0 00007ff6`4248d84c     WindowsTerminal!winrt::Windows::Foundation::IActivationFactory::ActivateInstance<winrt::TerminalApp::App>+0xc6 [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\base.h @ 6398] 
0e 00000048`ce0ff710 00007ff6`42497f3b     WindowsTerminal!<lambda_74300ac9aae6189311da41a1000f9c41>::operator()+0x3c [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\TerminalApp.h @ 4832] 
0f 00000048`ce0ff750 00007ff6`42499a1a     WindowsTerminal!<lambda_74300ac9aae6189311da41a1000f9c41>::<lambda_invoker_cdecl>+0x3b [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\TerminalApp.h @ 4832] 
10 00000048`ce0ff780 00007ff6`4248ffab     WindowsTerminal!winrt::impl::factory_cache_entry<winrt::TerminalApp::App,winrt::Windows::Foundation::IActivationFactory>::call<winrt::TerminalApp::App (__cdecl*)(winrt::Windows::Foundation::IActivationFactory const &)>+0x1ea [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\base.h @ 6231] 
11 00000048`ce0ff820 00007ff6`42490f69     WindowsTerminal!winrt::impl::call_factory_cast<winrt::TerminalApp::App (__cdecl*)(winrt::Windows::Foundation::IActivationFactory const &),winrt::TerminalApp::App,winrt::Windows::Foundation::IActivationFactory,<lambda_74300ac9aae6189311da41a1000f9c41> >+0xab [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\base.h @ 6270] 
12 00000048`ce0ff860 00007ff6`4249d26a     WindowsTerminal!winrt::TerminalApp::App::App+0x39 [C:\a\1\s\src\cascadia\WindowsTerminal\Generated Files\winrt\TerminalApp.h @ 4832] 
13 00000048`ce0ff8a0 00007ff6`42497c2b     WindowsTerminal!AppHost::AppHost+0x6a [C:\a\1\s\src\cascadia\WindowsTerminal\AppHost.cpp @ 28] 
14 00000048`ce0ff990 00007ff6`42491b62     WindowsTerminal!wWinMain+0x19b [C:\a\1\s\src\cascadia\WindowsTerminal\main.cpp @ 126] 
15 (Inline Function) --------`--------     WindowsTerminal!invoke_main+0x21 [D:\agent\_work\10\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 118] 
16 00000048`ce0ffaa0 00007ffa`9b517bd4     WindowsTerminal!__scrt_common_main_seh+0x106 [D:\agent\_work\10\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl @ 288] 
17 00000048`ce0ffae0 00007ffa`9cd0ce51     kernel32!BaseThreadInitThunk+0x14 [base\win32\client\thread.c @ 64] 
18 00000048`ce0ffb10 00000000`00000000     ntdll!RtlUserThreadStart+0x21 [minkernel\ntdll\rtlstrt.c @ 1153]

from dumps out of Helix from https://dev.azure.com/ms/terminal/_build/results?buildId=171754&view=results
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels May 28, 2021
@miniksa miniksa added Area-Build Issues pertaining to the build system, CI, infrastructure, meta Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Meta The product is the management of the products. labels May 28, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label May 28, 2021
@miniksa
Copy link
Member Author

miniksa commented May 28, 2021

Honestly I have no idea.

I suspect the SDK change or one of the other startup crashy things.

I'm basically bisecting the last 14ish days of commits by running assorted builds here (https://dev.azure.com/ms/terminal/_build?definitionId=492&_a=summary) while trying to get inspiration by staring at commits/PRs.

@miniksa miniksa self-assigned this May 28, 2021
@miniksa
Copy link
Member Author

miniksa commented May 28, 2021 •
edited
Loading

FAIL: 89ca2ae - Don't throw in GetProposedDimensions (#10260)
FAIL: 31d78dc - Ignore closeOnExit when a conn. moves from Connecting to Failed (#10263)
FAIL: 31e5880 - Add cooked data read tracing (#10166)
FAIL: 89af444 - Emit fixup debug info for internal tooling
FAIL: 504e610 - openConsole.psm1: fix unapproved verb warning
FAIL: eaeab7a - Upgrade Widnows SDK to 19041
PASS: c3bf8a5 - doc: Added links to folders and headers mentioned (#10111)

Looks like it broke with the SDK change...

@miniksa
Copy link
Member Author

miniksa commented Jun 8, 2021 

TerminalApp.dll!00007FFAFBFFCB67: ReturnHr(1) tid(f80) 8000FFFF Catastrophic failure
    Msg:[winrt::hresult_error: WindowsXamlManager and DesktopWindowXamlSource are supported for apps targeting Windows version 10.0.18226.0 and later.  Please check either the application manifest or package manifest and ensure the MaxTestedVersion property is updated.] 
(1904.f80): C++ EH exception - code e06d7363 (first chance)

Uhhh... it should be 10.0.19041.0 now which is greater...

@miniksa
Copy link
Member Author

miniksa commented Jun 8, 2021

Patching it to 10.0.18362.0 in the MaxVersionTested field of the embedded manifest makes it work again.

@miniksa miniksa mentioned this issue Jun 8, 2021
Merged
3 tasks
@ghost ghost added the In-PR This issue has a related PR label Jun 8, 2021
@ghost ghost closed this as completed in #10370 Jun 9, 2021
@ghost ghost removed the In-PR This issue has a related PR label Jun 9, 2021
ghost pushed a commit that referenced this issue Jun 9, 2021
@miniksa
Restore embedded manifests to say 18362 (#10370)
76d2aad 
Restore embedded manifests to say 18362 or unpackaged activation won't work (for helix testing.)

## PR Checklist
* [x] Closes #10265 
* [x] I work here
* [x] Tests now pass

## Detailed Description of the Pull Request / Additional comments
- Unpackaged activation uses the embedded manifest inside the exe. We use unpackaged activation to run our tests in Helix as it's easier that way. Turns out the 1903/19h1 OS thinks 19041 isn't greater than the minimum XAML islands version of 18226 and blocks the load of `TerminalApp.dll` causing a crash (fail fast) on launch. For **REASONS**, 18362 is considered greater than 18226. 
- Packaged activation will use the value in the .appxmanifest and everything is somehow still fine there even with it saying 19041 now.

## Validation Steps Performed
- Kicking a Helix-run off on this branch: https://dev.azure.com/ms/terminal/_build/results?buildId=177336&view=results
@ghost ghost added the Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. label Jun 9, 2021
@roxk
Copy link

roxk commented Sep 28, 2021

Hi, we were discussing this issue in the uwp community discord, and just realized the doc for maxversiontested had been updated. Apparently, the proper fix is to include multiple maxversiontested, e.g.

<maxversiontested Id="10.0.18362.0"/>
<maxversiontested id="10.0.19041.0"/>

So instead of treating it like an integer with which comparison is possible (e.g. 19041 > 18362), maxversiontested is a multi-valued string entry.

Seems like this can be fixed with the document way :) Or should I file a separate issue instead?

@miniksa
Copy link
Member Author

miniksa commented Sep 29, 2021

The problem is that a machine running 18362 will not recognize 19041 as a valid version that Windows can be and will exclude itself from allowing the package to run. I looked at this internally with the folks behind the code. They recommended that I just set it to 18362 for as long as we want to continue supporting 19H1 with the Terminal package.

@miniksa
Copy link
Member Author

miniksa commented Oct 2, 2021 •
edited
Loading

Sorry someone brought my attention to this. It looks like I misinterpreted you. I believe it is okay and probably more correct to have the multiple string values as you show, @roxk. But given that practically you only need the lowest one, we can't be bothered to keep track of inserting more of them and will only change it when we raise the minimum.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@miniksa miniksa

Labels
Area-Build Issues pertaining to the build system, CI, infrastructure, meta Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Meta The product is the management of the products. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Restore embedded manifests to say 18362 microsoft/terminal
2 participants
@roxk @miniksa