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

Allow VisualStudioExperimentationService to initialize on a background thread #30916

Merged
merged 2 commits into from
Nov 8, 2018

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Nov 1, 2018

Fixes #30892

Customer scenario

When the first Roslyn document opened in the editor is opened via the Navigate To feature, the Roslyn package fails to load and its IDE features are missing substantial functionality.

Bugs this fixes

#30892
https://devdiv.visualstudio.com/DevDiv/_workitems/edit/725190

Workarounds, if any

Open a source document before using Navigate To.

Risk

Low. In the event other initialization code has bugs (fails to follow required threading rules of the host application), the IDE could hang (deadlock) during the test scenario. However, this is unlikely to result in loss of data because changes cannot be made to most source documents prior to opening the first source document.

Performance impact

Code paths are not altered outside of the broken scenario.

Is this a regression from a previous update?

Unknown.

Root cause analysis

Manual testing scenarios should be updated to cover this case.

How was the bug found?

Internal report.

Test documentation updated?

Not yet.

@sharwell sharwell requested a review from a team as a code owner November 1, 2018 20:44
@CyrusNajmabadi
Copy link
Member

Is there a reason we need to have any blocking here? can we not just make the API async and allow getting this service through normal JTF means one demand?

@sharwell
Copy link
Member Author

sharwell commented Nov 2, 2018

@CyrusNajmabadi I will submit the asynchronous form as a separate pull request (#30929) targeting master, which we can merge after this simpler pull request moves forward

@jinujoseph
Copy link
Contributor

Approved to merge once green

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants