Factories no longer default to no_weak_ref #913
Merged
Conversation
This file contains hidden or 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
Originally, factories defaulted to no_weak_ref because there wasn't any use case for weak references to factories, and the thought was that the weak reference support would be a performance penalty. In practice, the weak reference support does not have a significant performance impact, so it's okay to leave it on even for factories. Allowing weak references to factories is important because factories implement the static members of a runtime class, and those members may require taking weak references: * Event handlers need weak references so that the event handler doesn't create a circular reference back to the factory. * Queueing work to a background thread takes a weak reference so that the work can be abandoned if the factory has been destroyed. (We don't want to extend the lifetime of COM objects beyond the lifetime of COM itself.) Furthermore, the ability to take a weak reference to a factory is required in order to use event_revoker for static events, since event_revoker requires that the event source support weak references.
oldnewthing
commented
Apr 2, 2021
| } | ||
|
|
||
| using is_factory = std::disjunction<std::is_same<Windows::Foundation::IActivationFactory, I>...>; | ||
|
|
Member
Author
There was a problem hiding this comment.
Deleting this public type is a potential breaking change. Do we care?
Collaborator
There was a problem hiding this comment.
I don't think it matters too much - developers can easily sniff this out themselves - and its not likely to be something a project would depend on, I think.
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.
Originally, factories defaulted to no_weak_ref because there wasn't believed to be any use case for weak references to factories, and the thought was that the weak reference support would be a performance penalty.
In practice, the weak reference support does not have a significant performance impact, so it's okay to leave it on even for factories.
Allowing weak references to factories is important because factories implement the static members of a runtime class, and those members may require taking weak references:
Furthermore, the ability to take a weak reference to a factory is required in order to use event_revoker for static events, since event_revoker requires that the event source support weak references.