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

Sequence operations to credentials and clean up better when setting #175695

Merged
merged 1 commit into from Feb 28, 2023

Conversation

TylerLeonhardt
Copy link
Member

Fixes #130893

Although this one has been challenging to reproduce, my suspicion is that users were hitting the following behavior:

  • multiple windows would open due to a VS Code update
  • multiple windows were now writing to SecretStorage
  • on Windows we have to chunk these writes into multiple writes, but we weren't sequentializing these writes so these two async operations writing to the same place asynchronously would cause the Cred Man to get into a bad state
  • User reloads and we read from secrets with chunks from two different SecretStorage#store() calls so we fail to parse the secrets
  • User is asked to sign in again
  • Additionally, those old chunks are left behind because we don't delete secrets when we fail to parse
  • Eventually a user hits a limit of Cred Man for the number of secrets in it which gets them in a worse state

So with this fix, we will:

  • Delete the old secret before writing the new one (in case they have secret there that is not the same number of chunks)
  • Make sure every write/delete/get is handled sequentially so that these operations don't cause the secret store to be in a bad state

@TylerLeonhardt TylerLeonhardt merged commit 35b8217 into main Feb 28, 2023
@TylerLeonhardt TylerLeonhardt deleted the tyler/wispy-buzzard branch February 28, 2023 22:07
@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants