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

Terminal can lock up when typing if the cursor is hovering a link #4341

Closed
Tyriar opened this issue Dec 20, 2022 · 2 comments · Fixed by #4342
Closed

Terminal can lock up when typing if the cursor is hovering a link #4341

Tyriar opened this issue Dec 20, 2022 · 2 comments · Fixed by #4342
Assignees
Labels
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Dec 20, 2022

VS Code issue: microsoft/vscode#169614

Repros in xterm.js:

  1. Write a link
  2. Hover it
  3. Type slowly, it will get exponentially slower each character
@Tyriar
Copy link
Member Author

Tyriar commented Dec 20, 2022

Looks like the cause is that any viewport change registers a new event listener that only gets disposed when the original link is disposed:

this._linkCacheDisposables.push(this._renderService.onRenderedViewportChange(e => {

@Tyriar
Copy link
Member Author

Tyriar commented Dec 20, 2022

This only happens when the link is on a different line to the cursor. This clear call doesn't end up clearing the old link otherwise, which is where the leak happens:

this._clearCurrentLink(start, e.end + 1 + this._bufferService.buffer.ydisp);

Changing this to this._handleHover(position) fixes the problem for example because it ends up calling this._clearCurrentLink(undefined, undefined) which clears all links:

this._askForLink(position, false);

https://github.com/Tyriar/xterm.js/blob/3a74930d072fe883c35ded5ee0052470ee44d470/src/browser/Linkifier2.ts#L328

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Dec 20, 2022
The bug here is that _clearCurrentLink was being called only on the
line(s) that changed, but _askForLink was then being triggered
regardless. This caused more onRenderedViewportChange listeners to be
registered and for the thread blocking to get exponentially worse.

Fixes xtermjs#4341
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant