Skip to content

Fix memory leak in GoImageProvider #74

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

Closed
wants to merge 1 commit into from
Closed

Fix memory leak in GoImageProvider #74

wants to merge 1 commit into from

Conversation

cookieo9
Copy link
Contributor

Qt uses "implicit shared memory" for many of its types which is a scheme
similar to copy on write, and is tracked by reference counting in the
constructors/destructor of its classes.

Unfortunately the image provider code creates an *QImage, which since it
wasn't delete'd the destructor is not called, so when QML is done with its
own copies, the shared image data can't be freed.

Qt uses "implicit shared memory" for many of its types which is a scheme
similar to copy on write, and is tracked by reference counting in the
constructors/destructor of its classes.

Unfortunately the imageprovider code creates an *QImage, which since it
wasn't delete'd the destructor is not called, so when QML is done with its
own copies, the shared image data can't be freed.
@cookieo9
Copy link
Contributor Author

Should I also do a patch for v0 (master)?

@niemeyer niemeyer closed this Aug 13, 2014
@cookieo9
Copy link
Contributor Author

Any reason this was closed? The memory leak still exists in v1.

@niemeyer
Copy link
Contributor

That's awkward.. I don't think I had even seen this issue, despite the fact it marks me as having closed, and I would definitely not have closed it without a comment.

@niemeyer
Copy link
Contributor

Ahh.. this is just a really bad UI. It must have closed it automatically because I deleted the v1-dev branch. I cannot reopen it either. Can you please repropose against v1?

@cookieo9
Copy link
Contributor Author

I have re-done the patch for v1, I should be able to make a pull request for it quickly.

@cookieo9
Copy link
Contributor Author

Done #88.

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