Hot (un)load extensions on (un)install #375
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
But it's probably overkill. I think I really only need the things in ExtensionService. The problem was that TopLevelCommandManager iterates over the installed extensions on a reload, but SIMULTANEOUSLY, the ExtensionService gets the notification that the extension was uninstalled, and it fuckin blows away the whole _installedExtensions cache
…://github.com/zadjii-msft/powertoys into dev/migrie/f/update-extensions-when-installed
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.
Closes #370
The DevHome code was great for "I need something that can lookup extensions and enumerate all of them".
However, the DevHome code is a very blunt hammer when it comes to extensions. The only thing it tracks is "packages changed", and if it gets one of those, it just blows away all the extensions and rebuilds them. Yikes.
This PR changes
ExtensionServiceto be a scalpel. We'll keep_installedExtensionsfresh. When we get a package install, we'll add only that package's extension to our cache, and let theTopLevelCommandManagerknow. Similarly for updates and uninstalls.That way, we can exactly change the top-level list as needed, rather than bluntly forcing all the extensions to reload.
In the middle of all this, I fixed a bug where uninstalling an extension, then reloading would just fail to load extensions. This is because the old code would clear out the whole list of extensions when one was uninstalled. That created a race where we'd be parsing the new list of all the extensions (from the reload), get an uninstall event, clear the list, then InvalidOperation as the list of extensions was modified during enumeration.
There's a bunch more locking in here. This might drive-by #324 but hard to be sure.
Related to #89