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

Multiple DebugEditorModels for same URI lead to stack overflow #11878

Closed
colin-grant-work opened this issue Nov 17, 2022 · 1 comment · Fixed by #12183
Closed

Multiple DebugEditorModels for same URI lead to stack overflow #11878

colin-grant-work opened this issue Nov 17, 2022 · 1 comment · Fixed by #12183
Labels
bug bugs found in the application debug issues that related to debug functionality

Comments

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Nov 17, 2022

Bug Description:

If two editors are open for the same file, a number of issues arise related to breakpoint management due to the fact that both editors have their own DebugEditorModels.

Steps to Reproduce:

  1. Open an editor.
  2. Open a second instance (Ctrl+\)
  3. Add a breakpoint.
  4. Right click on the breakpoint decoration in the first editor and say 'Edit Breakpoint'
  5. Observe that the breakpoint editor opens in the second editor.
  6. Move the breakpoint up or down a few lines by adding or removing content from the second editor.
  7. Observe hundreds of warnings about calling deltaDecorations recursively, as well as, possibly, a freeze and numerous stack overflow warnings.

Additional Information

image

There are two related issues. A DebugEditorModel is supposed to ignore events that originate from itself, which is the import of the check for this.updatingDecorations here:

protected areBreakpointsAffected(): boolean {
if (this.updatingDecorations || !this.editor.getControl().getModel()) {
return false;
}
for (const decoration of this.breakpointDecorations) {
const range = this.editor.getControl().getModel()!.getDecorationRange(decoration);
const oldRange = this.breakpointRanges.get(decoration)!;
if (!range || !range.equalsRange(oldRange)) {
return true;
}
}
return false;
}

But if another editor is active for the same URI, then each will trigger the other to check its breakpoints and eventually emit a new change event.

Relatedly, in the DebugEditorService, a new model is created for every editor, regardless of whether a model exists for that URI. This may also lead to undisposed DebugEditorModels.

protected push(widget: EditorWidget): void {
const { editor } = widget;
if (!(editor instanceof MonacoEditor)) {
return;
}
const uri = editor.getControl().getModel()!.uri.toString();
const debugModel = this.factory(editor);
this.models.set(uri, debugModel);
editor.getControl().onDidDispose(() => {
debugModel.dispose();
this.models.delete(uri);
});
}

  • Operating System: RHEL 7
  • Theia Version: fe8e4c1
@colin-grant-work colin-grant-work added bug bugs found in the application debug issues that related to debug functionality labels Nov 17, 2022
@colin-grant-work
Copy link
Contributor Author

The behavior seems to be quite bad in Firefox; it's hard to reproduce steps 6-7 at all in Chrome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application debug issues that related to debug functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant