Skip to content
This repository has been archived by the owner on Apr 14, 2022. It is now read-only.

Do not allow the analyzer to be reset if the server is still initializing #1778

Merged
merged 1 commit into from
Nov 8, 2019

Conversation

jakebailey
Copy link
Member

@jakebailey jakebailey commented Nov 8, 2019

For #1697, #1776.

@@ -50,6 +50,7 @@ public sealed partial class Server : IDisposable {
private IIndexManager _indexManager;

private InitializeParams _initParams;
private bool _initialized;
Copy link
Member Author

@jakebailey jakebailey Nov 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, this is racy. I can replace it with something else, but I'm not sure how worthwhile it is for a best-effort bool (which is probably atomic already...).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we default to false?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default boolean is false already.

@@ -249,6 +252,12 @@ public sealed partial class Server : IDisposable {
}

private void ResetAnalyzer() {
if (!_initialized) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another approach could be making this to be async method and await until initialized to happen and then proceed. but I saw caller of this thing and how things connected so might be too much work for a corner case.

basically, it looks like this can happen if files in path server watching (such as lib directory and etc) changed before initialization finished. and it those case, ignoring those changes seems fine to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we don't have to go overkill because if it's still being initialized, then any future reads of library files will necessarily be after them being changed, so we'll get the updated info anyway. Reloading is more of a "everything you have before now is wrong, reset", but before init this is meaningless.

@jakebailey jakebailey merged commit c5fb43a into microsoft:master Nov 8, 2019
@jakebailey jakebailey deleted the no-reload-before-init branch November 8, 2019 22:19
MikhailArkhipov pushed a commit that referenced this pull request Jun 15, 2020
MikhailArkhipov pushed a commit that referenced this pull request Jun 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants