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

Re #178715. Wait for notebook model to be ready for dispose #180560

Merged
merged 1 commit into from
Apr 21, 2023
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
12 changes: 12 additions & 0 deletions src/vs/workbench/contrib/notebook/common/notebookEditorModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,18 @@ export class SimpleNotebookEditorModel extends EditorModel implements INotebookE
return Boolean(this._workingCopy?.model?.notebookModel);
}

async canDispose(): Promise<boolean> {
if (!this._workingCopy) {
return true;
}

if (SimpleNotebookEditorModel._isStoredFileWorkingCopy(this._workingCopy)) {
return this._workingCopyManager.stored.canDispose(this._workingCopy);
} else {
return true;
}
}

isDirty(): boolean {
return this._workingCopy?.isDirty() ?? false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class NotebookModelReferenceCollection extends ReferenceCollection<Promise<IReso

private readonly _dirtyStates = new ResourceMap<boolean>();

private readonly modelsToDispose = new Set<string>();
constructor(
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@INotebookService private readonly _notebookService: INotebookService,
Expand All @@ -56,6 +57,9 @@ class NotebookModelReferenceCollection extends ReferenceCollection<Promise<IReso
}

protected async createReferencedObject(key: string, viewType: string, hasAssociatedFilePath: boolean): Promise<IResolvedNotebookEditorModel> {
// Untrack as being disposed
this.modelsToDispose.delete(key);

const uri = URI.parse(key);

const workingCopyTypeId = NotebookWorkingCopyTypeIdentifier.create(viewType);
Expand Down Expand Up @@ -101,14 +105,37 @@ class NotebookModelReferenceCollection extends ReferenceCollection<Promise<IReso
return result;
}

protected destroyReferencedObject(_key: string, object: Promise<IResolvedNotebookEditorModel>): void {
object.then(model => {
this._modelListener.get(model)?.dispose();
this._modelListener.delete(model);
model.dispose();
}).catch(err => {
this._logService.error('FAILED to destory notebook', err);
});
protected destroyReferencedObject(key: string, object: Promise<IResolvedNotebookEditorModel>): void {
this.modelsToDispose.add(key);

(async () => {
try {
const model = await object;

if (!this.modelsToDispose.has(key)) {
// return if model has been acquired again meanwhile
return;
}

if (model instanceof SimpleNotebookEditorModel) {
await model.canDispose();
}

if (!this.modelsToDispose.has(key)) {
// return if model has been acquired again meanwhile
return;
}

// Finally we can dispose the model
this._modelListener.get(model)?.dispose();
this._modelListener.delete(model);
model.dispose();
} catch (err) {
this._logService.error('FAILED to destory notebook', err);
} finally {
this.modelsToDispose.delete(key); // Untrack as being disposed
}
})();
}
}

Expand Down