-
Notifications
You must be signed in to change notification settings - Fork 308
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.
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
Unpackaged Windows App SDK app doesn't run on RS5/19H1 #1681
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…otstrap enumerates potential DDLMs via PackageManager.FindsPackagesByPackageType(framework) instead of AppExtension.OpenCatalog(ddlm), 2) Bootstrap uses a DesktopBridge application process instead of a PackagedCOM OOP Server to indicate WinAppSDK Framework package-in-use, and 3) the package-in-use process quits when it's told to (because MddBootstrapShutdown was called) or the caller process terminated (w/o calling MddBootstrapShutdown) instead of last COM reference to the PackagedCOM OOP Server was Release'd. We now get N 'shadow' processes (1 per Bootstrap-calling process) instead of 1 process (1 PackagedCOM OOP Server for all Bootstrap-calling processes), but the DDLMShadow process is small and cheap. Single-instancing DDLMShadow.exe is an exercise for the future (assuming it's not obsolete and dropped). NOTE: Still has some debugging code pending testing. At least 1 more commit to come...
ghost
added
the
needs-triage
label
Oct 29, 2021
DrusTheAxe
requested review from
Scottj1s,
MikeHillberg,
aeloros,
alexlamtest,
axelandrejs,
BenJKuhn,
EHO-makai,
evelynwu-msft,
huichen123 and
jonwis
October 29, 2021 19:45
huichen123
reviewed
Oct 29, 2021
dev/DynamicDependency/DynamicDependencyLifetimeManagerShadow/winmain.cpp
Show resolved
Hide resolved
Sounds like this is also related to #1121 |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
EHO-makai
reviewed
Oct 30, 2021
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
EHO-makai
approved these changes
Nov 1, 2021
Scottj1s
reviewed
Nov 1, 2021
dev/DynamicDependency/DynamicDependencyLifetimeManagerShadow/winmain.cpp
Outdated
Show resolved
Hide resolved
Scottj1s
reviewed
Nov 1, 2021
dev/DynamicDependency/DynamicDependencyLifetimeManagerShadow/winmain.cpp
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
jonwis
reviewed
Nov 1, 2021
dev/DynamicDependency/DynamicDependencyLifetimeManagerShadow/winmain.cpp
Show resolved
Hide resolved
Scottj1s
reviewed
Nov 1, 2021
Scottj1s
reviewed
Nov 1, 2021
Scottj1s
reviewed
Nov 1, 2021
Scottj1s
reviewed
Nov 1, 2021
Scottj1s
reviewed
Nov 1, 2021
P.S. The PR's titled RS5/19H1 but the problem's also on 19H2 i.e. Windows <20H1 is affected |
DrusTheAxe
added a commit
that referenced
this pull request
Nov 3, 2021
Non-AppExtension implementation for <=19H1 Altered Bootstrap to workaround AppExtension issue on RS5+19H1: 1) Bootstrap enumerates potential DDLMs via PackageManager.FindsPackagesByPackageType(framework) instead of AppExtension.OpenCatalog(ddlm) 2) Bootstrap uses a DesktopBridge application process instead of a PackagedCOM OOP Server to indicate WinAppSDK Framework package-in-use 3) the package-in-use process quits when it's told to (because MddBootstrapShutdown was called) or the caller process terminated (w/o calling MddBootstrapShutdown) instead of last COM reference to the PackagedCOM OOP Server was Release'd. We now get N 'shadow' processes (1 per Bootstrap-calling process) instead of 1 process (1 PackagedCOM OOP Server for all Bootstrap-calling processes), but the DDLMShadow process is small and cheap. Single-instancing DDLMShadow.exe is an exercise for the future (assuming it's not obsolete and dropped). Added/relies-on a 'release marker' file in the DDLM package to identify the major.minor release (required as otherwise few options to determine the major, none of them cheap).
DrusTheAxe
added a commit
that referenced
this pull request
Nov 3, 2021
…19h1 Unpackaged Windows App SDK app doesn't run on RS5/19H1 (#1681)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://task.ms/36777674
The bootstrapper relies on AppExtension to enumerate candidate DDLMs for the specified criteria. However AppExtension.Find*(name) only matches packages on <=19H1 if the caller is packaged and declares a matching AppExtensionHost (this must-have-matching-AppExtensionHost requirement was removed in 20H1). Since unpackaged apps have no
appxmanifest.xml
to declare an AppExtensionHostMddBootstrapInitalize()
fails on RS5+19H1.Bootstrap implementation is altered to workaround this AppExtension/RS5+19H1 limitation. The spec will be updated but here's the synopsis
PackageManager.FindsPackagesByPackageType(framework)
instead ofAppExtension.OpenCatalog(ddlm)
DynamicDependencyLifetimeManagerShadow.exe
) instead of a PackagedCOM OOP Server to inform Deployment the WinAppSDK Framework package-in-use...it's told to quit via an named event (i.e.
MddBootstrapShutdown
signals to it)or
...the caller process terminated (w/o calling
MddBootstrapShutdown
) instead of last COM reference to the PackagedCOM OOP Server was Release'dDDLMshadow waits on the named event (signal from MddBootstrapInitialize) and caller process handle (
OpenProcess
) and quits when either pops.MddBootstrapInitalize()
activates the DDLMshadow app with arguments including the caller's process id (so DDLMshadow can get a handle to the process) and a unique id (to ensure the caller process hasn't quit and processid reused for a new process before DDLMshadow uses it).This results in N DDLM processes (1 per Bootstrap-calling process) instead of 1 DDLM process (the PackagedCOM
OOP Server for all Bootstrap-calling processes). IOW the DDLMshadow app process is multi-instanced vs DDLM PackagedCOM process is single-instanced.
NOTE: Parts of this will be used to address the elevation issue (#567). More is needed to solve that but this work isn't entirely just-for-downlevel :-)
This "DDLMviaEnumeration" codepath is used instead of the DDLMviaAppExtension when running on <=19H1 -or- the environment variable
MICROSOFT_WINDOWSAPPRUNTIME_DDLM_ENUMERATION
exists (e.g.set MICROSOFT_WINDOWSAPPRUNTIME_DDLM_ENUMERATION=1
). The latter was used to verify same net behavior on >19H1 despite different implementation.All related UnitTests pass as expected.
Additional E2E tests are being added in the aggregator as a separate PR.
NOTE: This change requires the aggregator be updated adding a file to DDLM MSIX packages named
Microsoft.WindowsAppRuntime.Release=<rmajor>.<rminor>.<rpatch>
specifying the release version (e.g. 1.0.0). This is necessary as the MSIX package versioning scheme currently does not specify thermajor
in the package's identity. This was handled via AppExtension but lacking that is only available if we "Find, Load and Parse appxmanifest.xml" at runtime <shudder>. This file let's use much more efficiently determine the release major version, necessary so we match the expected release e.g. so `MddBootstrapInitialize(0x00010002,...) doesn't match 0.2., 2.2., ...