Mitigate memory leak and disable affected tests #761
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
All the disabled tests are associated with issue dotnet#759 which tracks re-enabling these.
|
Thanks, I'll work on fixing these asap. 👍 |
jasonmalinowski
added a commit
that referenced
this pull request
Feb 21, 2015
Mitigate memory leak and disable affected tests
mavasani
added a commit
that referenced
this pull request
Feb 22, 2015
My prior change #673 made to address issues with analyzer exception diagnostics introduced a few leaks in product and test code. The primary reason was that I attempted to use static events to track state as the existing AnalyzerDriverHelper type which did the core analyzer execution was a static type with all static methods. Additionally, I added a static event to the new host diagnostic update source added for reporting analyzer specific diagnostics. These were holding onto projects/workspaces in test runs causing leaks. I have reverted the approach of static events and instead refactored the code in AnalyzerDriver project to simplify the whole design: 1. Renamed AnalyzerDriverHelper to AnalyzerExecutor and made it a non-static type, which has instance fields for all the configuration parameters for analyzer callbacks. 2. Move all the core analyzer callbacks (actions/Initialize method/supported diagnostics) into AnalyzerExecutor. Command line compiler just creates a single instance of the executor, while IDE driver creates instances per analyzer. 3. Both these changes simplified the API a lot, and I just had to add an additional field "Action addExceptionDiagnostic" to AnalyzerExecutor to configure how to handle exception diagnostics. 4. MEF import AbstractHostDiagnosticUpdateSource in DiagnosticAnalyzerService and thread it down to IDE analyzer driver. 5. Changes 3 and 4 above meant that in the IDE driver, delegate "addExceptionDiagnostic" just asks the HostDiagnosticUpdateSource to report the exception diagnostic produced by the AnalyzerExecutor. This change also re-enables the tests skipped by #761 and fixes #759.
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.
All the disabled tests are associated with issue #759 which tracks
re-enabling these.