{"action":"created","issue":{"url":"https://api.github.com/repos/dotnet/roslyn/issues/24883","repository_url":"https://api.github.com/repos/dotnet/roslyn","labels_url":"https://api.github.com/repos/dotnet/roslyn/issues/24883/labels{/name}","comments_url":"https://api.github.com/repos/dotnet/roslyn/issues/24883/comments","events_url":"https://api.github.com/repos/dotnet/roslyn/issues/24883/events","html_url":"https://github.com/dotnet/roslyn/pull/24883","id":297652659,"number":24883,"title":"Fix SyntacticClassificationTagger's handling of workspace changes","user":{"login":"jasonmalinowski","id":201340,"avatar_url":"https://avatars3.githubusercontent.com/u/201340?v=4","gravatar_id":"","url":"https://api.github.com/users/jasonmalinowski","html_url":"https://github.com/jasonmalinowski","followers_url":"https://api.github.com/users/jasonmalinowski/followers","following_url":"https://api.github.com/users/jasonmalinowski/following{/other_user}","gists_url":"https://api.github.com/users/jasonmalinowski/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonmalinowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonmalinowski/subscriptions","organizations_url":"https://api.github.com/users/jasonmalinowski/orgs","repos_url":"https://api.github.com/users/jasonmalinowski/repos","events_url":"https://api.github.com/users/jasonmalinowski/events{/privacy}","received_events_url":"https://api.github.com/users/jasonmalinowski/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"jasonmalinowski","id":201340,"avatar_url":"https://avatars3.githubusercontent.com/u/201340?v=4","gravatar_id":"","url":"https://api.github.com/users/jasonmalinowski","html_url":"https://github.com/jasonmalinowski","followers_url":"https://api.github.com/users/jasonmalinowski/followers","following_url":"https://api.github.com/users/jasonmalinowski/following{/other_user}","gists_url":"https://api.github.com/users/jasonmalinowski/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonmalinowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonmalinowski/subscriptions","organizations_url":"https://api.github.com/users/jasonmalinowski/orgs","repos_url":"https://api.github.com/users/jasonmalinowski/repos","events_url":"https://api.github.com/users/jasonmalinowski/events{/privacy}","received_events_url":"https://api.github.com/users/jasonmalinowski/received_events","type":"User","site_admin":false},"assignees":[{"login":"jasonmalinowski","id":201340,"avatar_url":"https://avatars3.githubusercontent.com/u/201340?v=4","gravatar_id":"","url":"https://api.github.com/users/jasonmalinowski","html_url":"https://github.com/jasonmalinowski","followers_url":"https://api.github.com/users/jasonmalinowski/followers","following_url":"https://api.github.com/users/jasonmalinowski/following{/other_user}","gists_url":"https://api.github.com/users/jasonmalinowski/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonmalinowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonmalinowski/subscriptions","organizations_url":"https://api.github.com/users/jasonmalinowski/orgs","repos_url":"https://api.github.com/users/jasonmalinowski/repos","events_url":"https://api.github.com/users/jasonmalinowski/events{/privacy}","received_events_url":"https://api.github.com/users/jasonmalinowski/received_events","type":"User","site_admin":false}],"milestone":null,"comments":1,"created_at":"2018-02-16T01:11:54Z","updated_at":"2018-02-16T01:19:53Z","closed_at":null,"author_association":"MEMBER","pull_request":{"url":"https://api.github.com/repos/dotnet/roslyn/pulls/24883","html_url":"https://github.com/dotnet/roslyn/pull/24883","diff_url":"https://github.com/dotnet/roslyn/pull/24883.diff","patch_url":"https://github.com/dotnet/roslyn/pull/24883.patch"},"body":"The SyntacticClassificationTagger builds a queue of work items. When it sees a workspace change of ProjectChanged, it sees if it needs to enqueue a new parse due to options changing. If it sees a DocumentChange it also enqueues a new parse if it's the document (note: in active context) we are tracking. In all cases (and after these two) it also enqueues a task to ensure the _lastParsedDocument field is brought forward to the current version. That had asserts (and assumptions) that in that case the contents of the text editor would never change.\r\n\r\nThat assumption is not correct for two reasons:\r\n\r\n1. If a linked file is changing, we raise two DocumentChanged; both present the same OldSolution and NewSolution. If the non-active context gets its event raised first, then we would try to migrate the _lastParsedDocument forward, but that also is bringing forward a content change too.\r\n2. If a SolutionChanged event was raised (say for a refactoring) that also changed a document, we could get a document changing as well.\r\n\r\nThe code that migrated _lastParsedDocument, since it presumed the content didn't change, would then only set _lastParsedDocument but wouldn't set _lastParsedSnapshot. This meant things were out of sync,\r\nat least until the next parse that would reset them. I suspect this might be the cause of #23074.\r\n\r\nAs a fairly tactical fix, if the snapshots are different, I don't set _lastParsedDocument but rather enqueue a new parse operation. That way the two _lastParsed fields don't get out of sync and we still\r\nwill have a parse going that wil (eventually) refresh everything.\r\n\r\n<details><summary>Ask Mode template not completed</summary>\r\n\r\n<!-- This template is not always required. If you aren't sure about whether it's needed or want help filling out the sections,\r\nsubmit the pull request and then ask us for help. :) -->\r\n\r\n### Customer scenario\r\n\r\nWhat does the customer do to get into this situation, and why do we think this\r\nis common enough to address for this release.  (Granted, sometimes this will be\r\nobvious \"Open project, VS crashes\" but in general, I need to understand how\r\ncommon a scenario is)\r\n\r\n### Bugs this fixes\r\n\r\n(either VSO or GitHub links)\r\n\r\n### Workarounds, if any\r\n\r\nAlso, why we think they are insufficient for RC vs. RC2, RC3, or RTW\r\n\r\n### Risk\r\n\r\nThis is generally a measure our how central the affected code is to adjacent\r\nscenarios and thus how likely your fix is to destabilize a broader area of code\r\n\r\n### Performance impact\r\n\r\n(with a brief justification for that assessment (e.g. \"Low perf impact because no extra allocations/no complexity changes\" vs. \"Low\")\r\n\r\n### Is this a regression from a previous update?\r\n\r\n### Root cause analysis\r\n\r\nHow did we miss it?  What tests are we adding to guard against it in the future?\r\n\r\n### How was the bug found?\r\n\r\n(E.g. customer reported it vs. ad hoc testing)\r\n\r\n### Test documentation updated?\r\n\r\nIf this is a new non-compiler feature or a significant improvement to an existing feature, update https://github.com/dotnet/roslyn/wiki/Manual-Testing once you know which release it is targeting.\r\n\r\n</details>\r\n"},"comment":{"url":"https://api.github.com/repos/dotnet/roslyn/issues/comments/366117570","html_url":"https://github.com/dotnet/roslyn/pull/24883#issuecomment-366117570","issue_url":"https://api.github.com/repos/dotnet/roslyn/issues/24883","id":366117570,"user":{"login":"jasonmalinowski","id":201340,"avatar_url":"https://avatars3.githubusercontent.com/u/201340?v=4","gravatar_id":"","url":"https://api.github.com/users/jasonmalinowski","html_url":"https://github.com/jasonmalinowski","followers_url":"https://api.github.com/users/jasonmalinowski/followers","following_url":"https://api.github.com/users/jasonmalinowski/following{/other_user}","gists_url":"https://api.github.com/users/jasonmalinowski/gists{/gist_id}","starred_url":"https://api.github.com/users/jasonmalinowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jasonmalinowski/subscriptions","organizations_url":"https://api.github.com/users/jasonmalinowski/orgs","repos_url":"https://api.github.com/users/jasonmalinowski/repos","events_url":"https://api.github.com/users/jasonmalinowski/events{/privacy}","received_events_url":"https://api.github.com/users/jasonmalinowski/received_events","type":"User","site_admin":false},"created_at":"2018-02-16T01:19:53Z","updated_at":"2018-02-16T01:19:53Z","author_association":"MEMBER","body":"P.S; I'd also totally sign off on rewriting this type's WorkspaceChanged handler entirely to avoid this; this just seems like the most straightforward fix that gets me on my way."}}
{"actor":{"display_login":"jasonmalinowski"}}