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

ILinksList.dispose is never called #91536

Closed
akosyakov opened this issue Feb 26, 2020 · 3 comments
Closed

ILinksList.dispose is never called #91536

akosyakov opened this issue Feb 26, 2020 · 3 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@akosyakov
Copy link
Contributor

akosyakov commented Feb 26, 2020

We use it via Monaco and it's never called. Looking at the code, links are used only here:

const newLinks = list.links.map(link => new Link(link, provider));
but not dispose method. And then provider itself is registered as disposable, although LinkProvider does not have dispose method.

Do we overlook anything? If not it would mean that

dispose: () => {
if (typeof dto.id === 'number') {
this._proxy.$releaseDocumentLinks(handle, dto.id);
}
}
is never called either and the extension host process leaking links.

@jrieken
Copy link
Member

jrieken commented Feb 26, 2020

It's called here via this. Please add a repro sample

@jrieken jrieken added the info-needed Issue requires more information from poster label Feb 26, 2020
@jrieken jrieken self-assigned this Feb 26, 2020
@akosyakov
Copy link
Contributor Author

akosyakov commented Feb 26, 2020

It is reproducible with any tsconfig.json file with links.

You are right, provider is added as disposable, but only if it is disposable. Instead the original links list should be added:
Screenshot 2020-02-26 at 13 40 02
Screenshot 2020-02-26 at 13 40 06

When dispose is called, disposable storage is empty:
Screenshot 2020-02-26 at 13 40 54

@jrieken
Copy link
Member

jrieken commented Feb 26, 2020

You are right, provider is added as disposable, but only if it is disposable.

🤦‍♂ D'oh... Yes this shouldn't be the provider but the result of it...

@jrieken jrieken added bug Issue identified by VS Code Team member as probable bug and removed info-needed Issue requires more information from poster labels Feb 26, 2020
@jrieken jrieken added this to the February 2020 milestone Feb 26, 2020
@joaomoreno joaomoreno added the verified Verification succeeded label Feb 27, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants