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

Fixes two memory leaks, which caused the scraper to crash after a while.... #383

Merged
merged 1 commit into from Feb 10, 2015
Merged

Fixes two memory leaks, which caused the scraper to crash after a while.... #383

merged 1 commit into from Feb 10, 2015

Conversation

deadbeef84
Copy link
Contributor

... I believe this fixes #180.

Fixes #352 (duplicate).
Fixes #362 (duplicate).

The destructor for AsyncHandle needs to be virtual as its subclasses are
allocated dynamically. I believe this caused the ImageDownloadHandle and its
related resources (such as the HttpReq and its contents) not to be freed
correctly.

…le. I believe this fixes #180.

Fixes #352 (duplicate).
Fixes #362 (duplicate).

The destructor for AsyncHandle needs to be virtual as its subclasses are
allocated dynamically. I believe this caused the ImageDownloadHandle and its
related resources (such as the HttpReq and its contents) not to be freed
correctly.
Aloshi added a commit that referenced this pull request Feb 10, 2015
Fixes two memory leaks, which caused the scraper to crash after a while....
@Aloshi Aloshi merged commit 1de0b88 into Aloshi:master Feb 10, 2015
@Aloshi
Copy link
Owner

Aloshi commented Feb 10, 2015

Oh man, I never even thought about the destructor. I'm surprised I never got a compiler warning about this. Thanks so much!

@deadbeef84 deadbeef84 deleted the scraper-memleak-fix branch February 10, 2015 19:09
@deadbeef84
Copy link
Contributor Author

No worries! Thanks for the awesome software.

Yeah, I would expect compilers to warn about this too. Perhaps the warning level is too low for this to show up? I tried adding -Wall to the compiler flags, and it gave me lots of warnings - not sure if this was included though!

mmatyas pushed a commit to mmatyas/EmulationStation that referenced this pull request Mar 28, 2018
Limiting Last Played view to the last 50 games you played
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants