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): Memory leak in hyper-link extension #687

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

danielericlee
Copy link
Contributor

There's a memory leak in the hyper-link extension which will reliably crash a Chrome tab. To reproduce, go to to the documentation page in Chrome and paste in https://www.npmjs.com/package/@uiw/react-codemirror 3000-4000 times. You can paste them all at once or a couple hundred at a time.

Here's a recording of the crash:

codemirror-hyperlink-crash.mp4

The issue here is that when doc.toString() is in the while loop, a new string is created of the entire document, and Chrome creates a sliced string when running defaultRegexp.exec. That sliced string has a parent reference back to the string created by doc.toString(). That sliced string, with the giant parent reference, gets set as the url state on the HyperLinkIcon widget. The result is that Chrome has in-memory a copy of the entire document for each of the links in the document, and runs out of memory as the document gets large.

Here's what it looks like in a Chrome memory snapshot:

image

Doing the toString() once outside of the loop creates a single sliced string, and each widget will have a reference to that single string.

I was able to verify locally that this change resolves the memory leak. I was able to paste 100,000 links in my app that uses the hyperlink extension without issue.

@jaywcjlove jaywcjlove merged commit e10f426 into uiwjs:master Oct 2, 2024
jaywcjlove added a commit that referenced this pull request Oct 2, 2024
@jaywcjlove
Copy link
Member

@danielericlee thx!

github-actions bot pushed a commit that referenced this pull request Oct 2, 2024
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.

2 participants