Skip to content

Fix link event listener leak#4342

Merged
Tyriar merged 2 commits intoxtermjs:masterfrom
Tyriar:4341
Dec 20, 2022
Merged

Fix link event listener leak#4342
Tyriar merged 2 commits intoxtermjs:masterfrom
Tyriar:4341

Conversation

@Tyriar
Copy link
Copy Markdown
Member

@Tyriar Tyriar commented 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 #4341

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
@Tyriar Tyriar added this to the 5.2.0 milestone Dec 20, 2022
@Tyriar Tyriar requested a review from jerch December 20, 2022 23:01
@Tyriar Tyriar self-assigned this Dec 20, 2022
@Tyriar Tyriar marked this pull request as ready for review December 20, 2022 23:01
@Tyriar Tyriar enabled auto-merge December 20, 2022 23:02
@Tyriar Tyriar merged commit 75075eb into xtermjs:master Dec 20, 2022
@Tyriar Tyriar deleted the 4341 branch December 20, 2022 23:15
@jerch
Copy link
Copy Markdown
Member

jerch commented Dec 21, 2022

Oh wow - kudos to your test batteries finding this one. 😱 🙈

@Tyriar
Copy link
Copy Markdown
Member Author

Tyriar commented Dec 21, 2022

@jerch yeah it took a while to figure this one out, also spotted a race while digging into it microsoft/vscode#169696

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants