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

Handle already released source files that dont match scriptKind #54401

Merged
merged 1 commit into from
May 25, 2023

Conversation

sheetalkamat
Copy link
Member

In LS we have document registry which maintain the cache by the scriptKind but there is not such concept at program level.
So when we try to reuse program there is no fail-safe way to check if scriptKind doesn't match between program reuse unless we create the new source files.
But this also means that we could try to get new sourceFile two times from the host.

  1. when trying to see if we can reuse the program structure using old source file list and checking if new source file makes it such that program structure can be reused or does it need to be constructed from scratch.
  2. When we decide that we cant reuse the structure so we try to build program by scratch by processing root files, dependencies etc.

In LS when we try to get the source file, we see if we already have the source file from old program and if yes and their script kinds don't match we just get updated source file from the document registry,
But if the old Source file's scriptKind doesn't match then we release that source file (removing our ref count) and acquiring the new source file with correct script kind.
What this resulted was, when we try to create source file when trying to determine if program structure can be reused, we would release the old document with script kind and acquire new document with correct script kind. But when the program structure is not reused, we would try to release the document with old Script kind but the bucket had the document with new script kind.

Fixes #54381

@mjbvz
Copy link
Contributor

mjbvz commented May 25, 2023

Awesome investigation @sheetalkamat ! Thank you for looking into this

@RyanCavanaugh Would we be able to get this in a TS 5.1.x recovery build? Copilot seems to be triggering this crash a fair amount by opening and closing documents quickly, but it's not a copilot specific issue

@sheetalkamat sheetalkamat merged commit c64a378 into main May 25, 2023
20 checks passed
@sheetalkamat sheetalkamat deleted the scriptKind branch May 25, 2023 22:58
@jakebailey
Copy link
Member

@typescript-bot cherry-pick this to release-5.1

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 25, 2023

Heya @jakebailey, I've started to run the task to cherry-pick this into release-5.1 on this PR at 8f0c69f. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

Hey @jakebailey, I've opened #54403 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request May 25, 2023
Component commits:
8f0c69f Handle already released source files that dont match scriptKind Fixes microsoft#54381
DanielRosenwasser pushed a commit that referenced this pull request May 26, 2023
…e-5.1 (#54403)

Co-authored-by: Sheetal Nandi <shkamat@microsoft.com>
Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
4 participants