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

fix leaking disposables #237586

Merged
merged 2 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/vs/editor/common/languages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2107,7 +2107,7 @@ export interface CodeLens {

export interface CodeLensList {
lenses: CodeLens[];
dispose(): void;
dispose?(): void;
}

export interface CodeLensProvider {
Expand Down
10 changes: 5 additions & 5 deletions src/vs/editor/common/languages/languageConfigurationRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { Emitter, Event } from '../../../base/common/event.js';
import { Disposable, IDisposable, toDisposable } from '../../../base/common/lifecycle.js';
import { Disposable, IDisposable, markAsSingleton, toDisposable } from '../../../base/common/lifecycle.js';
import * as strings from '../../../base/common/strings.js';
import { ITextModel } from '../model.js';
import { DEFAULT_WORD_REGEXP, ensureValidWordDefinition } from '../core/wordHelper.js';
Expand Down Expand Up @@ -202,15 +202,15 @@ class ComposedLanguageConfiguration {
);
this._entries.push(entry);
this._resolved = null;
return toDisposable(() => {
return markAsSingleton(toDisposable(() => {
for (let i = 0; i < this._entries.length; i++) {
if (this._entries[i] === entry) {
this._entries.splice(i, 1);
this._resolved = null;
break;
}
}
});
}));
}

public getResolvedConfiguration(): ResolvedLanguageConfiguration | null {
Expand Down Expand Up @@ -332,10 +332,10 @@ export class LanguageConfigurationRegistry extends Disposable {
const disposable = entries.register(configuration, priority);
this._onDidChange.fire(new LanguageConfigurationChangeEvent(languageId));

return toDisposable(() => {
return markAsSingleton(toDisposable(() => {
disposable.dispose();
this._onDidChange.fire(new LanguageConfigurationChangeEvent(languageId));
});
}));
}

public getLanguageConfiguration(languageId: string): ResolvedLanguageConfiguration | null {
Expand Down
2 changes: 1 addition & 1 deletion src/vs/editor/contrib/codelens/browser/codeLensCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class CodeLensCache implements ICodeLensCache {
};
});
const copyModel = new CodeLensModel();
copyModel.add({ lenses: copyItems, dispose: () => { } }, this._fakeProvider);
copyModel.add({ lenses: copyItems }, this._fakeProvider);

const item = new CacheItem(model.getLineCount(), copyModel);
this._cache.set(model.uri.toString(), item);
Expand Down
13 changes: 8 additions & 5 deletions src/vs/editor/contrib/codelens/browser/codelens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { CancellationToken } from '../../../../base/common/cancellation.js';
import { illegalArgument, onUnexpectedExternalError } from '../../../../base/common/errors.js';
import { DisposableStore } from '../../../../base/common/lifecycle.js';
import { DisposableStore, isDisposable } from '../../../../base/common/lifecycle.js';
import { assertType } from '../../../../base/common/types.js';
import { URI } from '../../../../base/common/uri.js';
import { ITextModel } from '../../../common/model.js';
Expand All @@ -24,18 +24,21 @@ export class CodeLensModel {

lenses: CodeLensItem[] = [];

private readonly _disposables = new DisposableStore();
private _store: DisposableStore | undefined;

dispose(): void {
this._disposables.dispose();
this._store?.dispose();
}

get isDisposed(): boolean {
return this._disposables.isDisposed;
return this._store?.isDisposed ?? false;
}

add(list: CodeLensList, provider: CodeLensProvider): void {
this._disposables.add(list);
if (isDisposable(list)) {
this._store ??= new DisposableStore();
this._store.add(list);
}
for (const symbol of list.lenses) {
this.lenses.push({ symbol, provider });
}
Expand Down
3 changes: 2 additions & 1 deletion src/vs/editor/contrib/hover/browser/contentHoverRendered.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ class RenderedContentHoverParts extends Disposable {
...hoverContext
};
const disposables = new DisposableStore();
disposables.add(statusBar);
for (const participant of participants) {
const renderedHoverParts = this._renderHoverPartsForParticipant(hoverParts, participant, hoverRenderingContext);
disposables.add(renderedHoverParts);
Expand All @@ -294,7 +295,7 @@ class RenderedContentHoverParts extends Disposable {
actions: renderedStatusBar.actions,
});
}
return toDisposable(() => { disposables.dispose(); });
return disposables;
}

private _renderHoverPartsForParticipant(hoverParts: IHoverPart[], participant: IEditorHoverParticipant<IHoverPart>, hoverRenderingContext: IEditorHoverRenderContext): IRenderedHoverParts<IHoverPart> {
Expand Down
32 changes: 19 additions & 13 deletions src/vs/editor/contrib/links/browser/getLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export class Link implements ILink {

export class LinksList {

static readonly Empty = new LinksList([]);

readonly links: Link[];

private readonly _disposables = new DisposableStore();
Expand Down Expand Up @@ -137,27 +139,31 @@ export class LinksList {

}

export function getLinks(providers: LanguageFeatureRegistry<LinkProvider>, model: ITextModel, token: CancellationToken): Promise<LinksList> {

export async function getLinks(providers: LanguageFeatureRegistry<LinkProvider>, model: ITextModel, token: CancellationToken): Promise<LinksList> {
const lists: [ILinksList, LinkProvider][] = [];

// ask all providers for links in parallel
const promises = providers.ordered(model).reverse().map((provider, i) => {
return Promise.resolve(provider.provideLinks(model, token)).then(result => {
const promises = providers.ordered(model).reverse().map(async (provider, i) => {
try {
const result = await provider.provideLinks(model, token);
if (result) {
lists[i] = [result, provider];
}
}, onUnexpectedExternalError);
});

return Promise.all(promises).then(() => {
const result = new LinksList(coalesce(lists));
if (!token.isCancellationRequested) {
return result;
} catch (err) {
onUnexpectedExternalError(err);
}
result.dispose();
return new LinksList([]);
});

await Promise.all(promises);

let res = new LinksList(coalesce(lists));

if (token.isCancellationRequested) {
res.dispose();
res = LinksList.Empty;
}

return res;
}


Expand Down
2 changes: 1 addition & 1 deletion src/vs/monaco.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8060,7 +8060,7 @@ declare namespace monaco.languages {

export interface CodeLensList {
lenses: CodeLens[];
dispose(): void;
dispose?(): void;
}

export interface CodeLensProvider {
Expand Down
10 changes: 5 additions & 5 deletions src/vs/platform/actions/common/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

import { IAction, SubmenuAction } from '../../../base/common/actions.js';
import { Event, MicrotaskEmitter } from '../../../base/common/event.js';
import { DisposableStore, dispose, IDisposable, toDisposable } from '../../../base/common/lifecycle.js';
import { DisposableStore, dispose, IDisposable, markAsSingleton, toDisposable } from '../../../base/common/lifecycle.js';
import { LinkedList } from '../../../base/common/linkedList.js';
import { ThemeIcon } from '../../../base/common/themables.js';
import { ICommandAction, ICommandActionTitle, Icon, ILocalizedString } from '../../action/common/action.js';
Expand Down Expand Up @@ -397,11 +397,11 @@ export const MenuRegistry: IMenuRegistry = new class implements IMenuRegistry {
this._commands.set(command.id, command);
this._onDidChangeMenu.fire(MenuRegistryChangeEvent.for(MenuId.CommandPalette));

return toDisposable(() => {
return markAsSingleton(toDisposable(() => {
if (this._commands.delete(command.id)) {
this._onDidChangeMenu.fire(MenuRegistryChangeEvent.for(MenuId.CommandPalette));
}
});
}));
}

getCommand(id: string): ICommandAction | undefined {
Expand All @@ -422,10 +422,10 @@ export const MenuRegistry: IMenuRegistry = new class implements IMenuRegistry {
}
const rm = list.push(item);
this._onDidChangeMenu.fire(MenuRegistryChangeEvent.for(id));
return toDisposable(() => {
return markAsSingleton(toDisposable(() => {
rm();
this._onDidChangeMenu.fire(MenuRegistryChangeEvent.for(id));
});
}));
}

appendMenuItems(items: Iterable<{ id: MenuId; item: IMenuItem | ISubmenuItem }>): IDisposable {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/platform/commands/common/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { Emitter, Event } from '../../../base/common/event.js';
import { Iterable } from '../../../base/common/iterator.js';
import { IJSONSchema } from '../../../base/common/jsonSchema.js';
import { IDisposable, toDisposable } from '../../../base/common/lifecycle.js';
import { IDisposable, markAsSingleton, toDisposable } from '../../../base/common/lifecycle.js';
import { LinkedList } from '../../../base/common/linkedList.js';
import { TypeConstraint, validateConstraints } from '../../../base/common/types.js';
import { ILocalizedString } from '../../action/common/action.js';
Expand Down Expand Up @@ -121,7 +121,7 @@ export const CommandsRegistry: ICommandRegistry = new class implements ICommandR
// tell the world about this command
this._onDidRegisterCommand.fire(id);

return ret;
return markAsSingleton(ret);
}

registerCommandAlias(oldId: string, newId: string): IDisposable {
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/contrib/extensions/browser/extensionsList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class Renderer implements IPagedRenderer<IExtension, ITemplateData> {
focusOnlyEnabledItems: true
});
actionbar.setFocusable(false);
actionbar.onDidRun(({ error }) => error && this.notificationService.error(error));
const actionBarListener = actionbar.onDidRun(({ error }) => error && this.notificationService.error(error));

const extensionStatusIconAction = this.instantiationService.createInstance(ExtensionStatusAction);
const actions = [
Expand Down Expand Up @@ -150,7 +150,7 @@ export class Renderer implements IPagedRenderer<IExtension, ITemplateData> {
const extensionContainers: ExtensionContainers = this.instantiationService.createInstance(ExtensionContainers, [...actions, ...widgets]);

actionbar.push(actions, { icon: true, label: true });
const disposable = combinedDisposable(...actions, ...widgets, actionbar, extensionContainers);
const disposable = combinedDisposable(...actions, ...widgets, actionbar, actionBarListener, extensionContainers);

return {
root, element, icon, name, installCount, ratings, description, publisherDisplayName, disposables: [disposable], actionbar,
Expand Down
11 changes: 9 additions & 2 deletions src/vs/workbench/contrib/scm/browser/scmViewPane.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { IContextKeyService, IContextKey, ContextKeyExpr, RawContextKey } from '
import { ICommandService } from '../../../../platform/commands/common/commands.js';
import { IKeybindingService } from '../../../../platform/keybinding/common/keybinding.js';
import { MenuItemAction, IMenuService, registerAction2, MenuId, IAction2Options, MenuRegistry, Action2, IMenu } from '../../../../platform/actions/common/actions.js';
import { IAction, ActionRunner, Action, Separator, IActionRunner } from '../../../../base/common/actions.js';
import { IAction, ActionRunner, Action, Separator, IActionRunner, toAction } from '../../../../base/common/actions.js';
import { ActionBar, IActionViewItemProvider } from '../../../../base/browser/ui/actionbar/actionbar.js';
import { IThemeService, IFileIconTheme } from '../../../../platform/theme/common/themeService.js';
import { isSCMResource, isSCMResourceGroup, isSCMRepository, isSCMInput, collectContextMenuActions, getActionViewItemProvider, isSCMActionButton, isSCMViewService, isSCMResourceNode, connectPrimaryMenu } from './util.js';
Expand Down Expand Up @@ -2988,7 +2988,14 @@ export class SCMActionButton implements IDisposable {
for (let index = 0; index < button.secondaryCommands.length; index++) {
const commands = button.secondaryCommands[index];
for (const command of commands) {
actions.push(new Action(command.id, command.title, undefined, true, async () => await this.executeCommand(command.id, ...(command.arguments || []))));
actions.push(toAction({
id: command.id,
label: command.title,
enabled: true,
run: async () => {
await this.executeCommand(command.id, ...(command.arguments || []));
}
}));
}
if (commands.length) {
actions.push(new Separator());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ export class AuthenticationUsageService extends Disposable implements IAuthentic
}
}

this._authenticationService.onDidRegisterAuthenticationProvider(
this._register(this._authenticationService.onDidRegisterAuthenticationProvider(
provider => this._queue.queue(
() => this._addExtensionsToCache(provider.id)
)
);
));
}

async initializeExtensionUsageCache(): Promise<void> {
Expand Down
Loading