[1.6 servicing] EnsurePackage*Async() handling options.RegisterNewerIfAvailable(true). RegisterPackageByPackageFull/FamilyNameAsync crash. Test + Warning fixes (#4845) #4864
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
…. RegisterPackageByPackageFull/FamilyNameAsync crash. Test + Warning fixes (#4845) EnsurePackage*Async() not handling options.RegisterNewerIfAvailable(true) RegisterPackageByPackageFullNameAsync() and RegisterPackageByPackageFullNameAsync() take their target parameter as const hstring& resulting in the reference captured by their implementations' co_await and not the actual value. The caller deallocates the memory before Register... is done with it potentially leading to BadMojo(TM). Tests don't notice it as they're too simple to notice this use-after-free -- the memory may be deallocated but the memory's not overwritten (yet) in simple loads (like the tests). Larger scale (real world) use with more diverse memory patterns notice the problem (usually as a crash). ApplicationDataTests_Elevated test cases were blocked state due to user context vs environmental state vs access control (ACLs whee!). Changed RunAs and associated test process setup to play nice together. Fixed some PackageManagerTests that had incomplete state setup potentially causing false errors if a past previous test failed. Fixed 400+ build warnings (mostly CS8305 due to [Experimental] attribute, almost all from OAuth generated C#/WinRT) Removed redundant .props import (fixing a VS warning) Fixed misleading log message Removed wrong test cases Added diagnostics to all Setup fixtures to hunt down Setup failures Change IsReady tests to RunAs RestrictedUser Fixed compiler warnings Split ApplicationDataTests_Elevated to separate source file. Added diagnostics to ApplicationDataTests[_Elevated]. Moved TAEF display+compare support for C++/WinRT hstring into shared file (test/inc/WindowsAppRuntime.Test.TAEF.cppwinrt.h). Added Parameters to ApplicationData tests to (hopefully) troubleshoot failure. Added whomi /all to build pipeline test output. Changed ApplicationData tests' default to IsolationLevel=Class (not TAEF's default =Module) Changed PackageManager tests' default to IsolationLevel=Class (not TAEF's default =Module) Factored tests into separate runs to aid troubleshooting failures Added suppression of CS8305 (Feature is experimental... duh) as not helpful and negatively helpful (400+ warnings that are, for us, features). https://task.ms/54835036 https://task.ms/54858998 https://task.ms/54884960
DrusTheAxe
added
the
area-Deployment
|
/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). |
codendone
reviewed
Nov 8, 2024
...soft.Security.Authentication.OAuth/Microsoft.Security.Authentication.OAuth.Projection.csproj
Outdated
Show resolved
Hide resolved
codendone
reviewed
Nov 8, 2024
...nModel.Background.Projection/Microsoft.Windows.ApplicationModel.Background.Projection.csproj
Outdated
Show resolved
Hide resolved
codendone
reviewed
Nov 8, 2024
...Microsoft.Windows.Media.Capture.Projection/Microsoft.Windows.Media.Capture.Projection.csproj
Outdated
Show resolved
Hide resolved
|
/azp run |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
codendone
approved these changes
Nov 8, 2024
codendone
approved these changes
Nov 8, 2024
|
/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). |
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.
EnsurePackage*Async() not handling options.RegisterNewerIfAvailable(true)
RegisterPackageByPackageFullNameAsync() and RegisterPackageByPackageFullNameAsync() take their target parameter as const hstring& resulting in the reference captured by their implementations' co_await and not the actual value. The caller deallocates the memory before Register... is done with it potentially leading to BadMojo(TM). Tests don't notice it as they're too simple to notice this use-after-free -- the memory maybe deallocated but the memory's not overwritten (yet) in simple loads (like the tests). Larger scale (real world) use with more diverse memory patterns notice the problem (usually as a crash).
ApplicationDataTests_Elevated test cases were blocked state due to user context vs environmental statevs access control (ACLs whee!). Changed RunAs and associated test process setup to play nice together.
Fixed some PackageManagerTests that had incomplete state setup potentially causing false errors if a past previous test failed.
Fixed 400+ build warnings (mostly CS8305 due to [Experimental] attribute, almost all from OAuth generat
ed C#/WinRT)
Removed redundant .props import (fixing a VS warning)
Fixed misleading log message
Removed wrong test cases
Added diagnostics to all Setup fixtures to hunt down Setup failures
Change IsReady tests to RunAs RestrictedUser
Fixed compiler warnings
Split ApplicationDataTests_Elevated to separate source file. Added diagnostics to ApplicationDataTests[_Elevated]. Moved TAEF display+compare support for C++/WinRT hstring into shared file (test/inc/WindowsAppRuntime.Test.TAEF.cppwinrt.h). Added Parameters to ApplicationData tests to (hopefully) troubleshoot failure.
Added whomi /all to build pipeline test output. Changed ApplicationData tests' default to IsolationLevel=Class (not TAEF's default =Module)
Changed PackageManager tests' default to IsolationLevel=Class (not TAEF's default =Module)
Factored tests into separate runs to aid troubleshooting failures
Added suppression of CS8305 (Feature is experimental... duh) as not helpful and negatively helpful (400+ warnings that are, for us, features).
https://task.ms/54835036
https://task.ms/54858998
https://task.ms/54884960
cherry-pick 60a18ba