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.

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

CommandBarFlyout static initialization creates winrt objects using COM API prohibited during DllMain() #6826

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

kmahone
Copy link
Member

@kmahone kmahone commented Mar 14, 2022

This is an issue that was caught by AppVerifier.
When Microsoft.UI.Xaml.dll is loaded, during DllMain is instantiates winrt objects, which is invalid. This is due to a pair of static arrays defined on CommandBarFlyout that were adding in PR #5347.

The fix is to initialize these arrays to nullptr, and populate them on the first creation of a CommandBarFlyout in its ctor.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Mar 14, 2022
@kmahone kmahone requested a review from RBrid March 14, 2022 20:05
// IsCompact and LabelPosition have no effect on an AppBarButton's rendering, when used as a secondary command, they are not present in the list.
// These two arrays are initialized in the constructor instead of being statically initialized here because that would result in the initialization happening during
// dllmain and it is not valid to call COM apis at that time.
winrt::DependencyProperty CommandBarFlyout::s_appBarButtonDependencyProperties[s_commandBarElementDependencyPropertiesCount]{ nullptr, nullptr, nullptr };
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{ nullptr, nullptr, nullptr };

I don't love that this is required to initialize the array to nullptrs.
It is not possible to just use "{}", because the default construction of a winrt::Foo object is not null - it creates a new instance of the object. Or at least it does if the winrt type itself has a default constructor. DependencyProperty does not have a constructor, since they are never instantiated directly (instead they are returned by DependencyProperty::Register). So in this case "{}" would be a compiler error.

@kmahone
Copy link
Member Author

kmahone commented Mar 14, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kmahone kmahone merged commit 60de1be into main Mar 14, 2022
@kmahone kmahone deleted the user/kmahone/cmdbarappverif branch March 14, 2022 21:44
@ojhad ojhad added area-DevInternal Internal build infrastructure, code cleanup, engineering efficiency team-Controls Issue for the Controls team area-CommandBarFlyout and removed needs-triage Issue needs to be triaged by the area owners labels Mar 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CommandBarFlyout area-DevInternal Internal build infrastructure, code cleanup, engineering efficiency team-Controls Issue for the Controls team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants