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

Memory leak of text & app crash on encode('png') #412

Closed
MarosPistej opened this issue Jan 27, 2022 · 5 comments · Fixed by #532
Closed

Memory leak of text & app crash on encode('png') #412

MarosPistej opened this issue Jan 27, 2022 · 5 comments · Fixed by #532
Assignees
Labels
bug Something isn't working

Comments

@MarosPistej
Copy link

MarosPistej commented Jan 27, 2022

Discovered memory leak while drawing text and segmentation error app crash when encoding

I created minimum reproducible example repository here https://github.com/MarosPistej/napi-rs-canvas-text-mem-leak

@MarosPistej MarosPistej changed the title Memory leak of text, rect, arc, bezier maybe more Memory leak of text & app crash on encode('png') Jan 27, 2022
@Brooooooklyn Brooooooklyn self-assigned this Jan 28, 2022
@Brooooooklyn Brooooooklyn added the bug Something isn't working label Jan 28, 2022
@louisfoster
Copy link

I have also encountered this leak, I think the encoding crash might be a separate issue (related to how skia handles the underlying buffer, for example if you pass the buffer to another tool like sharp without copying it first, then you'll get the seg error). Also the leak can occur even when the encoding step doesn't take place.

@louisfoster
Copy link

I'm wondering if paragraph needs to have reset() called at the end? I'm not a pro at C++, so I'm just trying to understand the cause of the leak, but looking at how paragraph is created it appears to use make_unique, which from my understanding is a kind of smarter/safer alternative to new. Although, reading this SO question, it is pointed out that something created via make_unique should subsequently called reset() to free the memory. However, I noticed in the code in my first link, there's no call to reset.

@louisfoster
Copy link

louisfoster commented Feb 10, 2022

I may have been slightly incorrect, but this reference says that if release() is called, something else needs to take care of deleting the object with delete.

@Brooooooklyn
Copy link
Owner

/cc @doodlewind

@louisfoster
Copy link

I tried adding delete paragraph; to the end of the function and tried building again. I didn't notice any difference in the memory with my tests, too much memory still being used and not released. In fact I believe I was also incorrect earlier about encode not also having some kind of leak, but it could possibly be my code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants