Angband.oook.cz
Angband.oook.cz
AboutVariantsLadderForumCompetitionComicScreenshotsFunniesLinks

Go Back   Angband Forums > Angband > Development

Reply
 
Thread Tools Display Modes
Old April 15, 2015, 11:26   #1
PowerWyrm
Prophet
 
PowerWyrm's Avatar
 
Join Date: Apr 2008
Posts: 2,672
PowerWyrm is on a distinguished road
[Angband 4.0] Massive memory leaks + possible double-free on object pointers

Oh dear...

Since the restructure, all object-related structures (object_kind/artifact/ego_item/object) have slays and brands as linked lists.

These structures can be described as:

struct obj_related
{
static fields;
static pointers; (kind, name...)
dynamic pointers; (slays, brands...)
};

Everywhere in the code, these structures are copied and wiped. For all static fields/pointers, this has no impact. But for dynamic pointers, the result is catastrophic.

Case #1: WIPE()

This simply does a memset(0) on the structure, which will set to zero every bit of the structure, including dynamic pointers. Basically, the memory allocated to these pointers is lost.

To avoid these memory leaks, all dynamic pointers must be freed before calling WIPE(). See object_wipe() for a good example.

Case #2: COPY()

This will do a bit-to-bit copy of the structures, which will copy everything including dynamic pointers. Here, the danger is double: not only the memory allocated to the dynamic pointers associated with the destination is lost (pointer address is replaced), but if you free the source object (assuming the dynamic pointers are properly freed when freeing the source object), you also free the dynamic pointers associated with the destination (since they are now equal)... and you could be in trouble when freeing the destination object (potential double-free of dynamic pointers).

To avoid this problem, all dynamic pointers must be freed before calling COPY, set to NULL after the copy and then copied separately. See object_copy() for a good example.

Case #3: structure copy

This is similar to case #2, as doing struct 2 = struct 1 is the same as doing COPY(&struct 2, &struct 1, struct). Same fix must apply. See scramble_artifact() for a good example of structure copy.
__________________
PWMAngband variant maintainer - check http://powerwyrm.monsite-orange.fr (or http://www.mangband.org/forum/viewforum.php?f=9) to learn more about this new variant!
PowerWyrm is offline   Reply With Quote
Old April 15, 2015, 11:58   #2
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 54
Posts: 7,862
Donated: $60
Nick will become famous soon enough
This was indeed a problem, but I believe it is all fixed in the latest code. I've done a lot of tracking down of leaks with valgrind.
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Old April 15, 2015, 12:14   #3
AnonymousHero
Veteran
 
AnonymousHero's Avatar
 
Join Date: Jun 2007
Posts: 1,367
AnonymousHero is on a distinguished road
Quote:
Originally Posted by Nick View Post
This was indeed a problem, but I believe it is all fixed in the latest code. I've done a lot of tracking down of leaks with valgrind.
Sorry to keep harping on about it, but may I yet again recommend AddressSanitizer? It gives much more precise information and it's fast enough to use all the time.
AnonymousHero is offline   Reply With Quote
Old April 15, 2015, 13:02   #4
PowerWyrm
Prophet
 
PowerWyrm's Avatar
 
Join Date: Apr 2008
Posts: 2,672
PowerWyrm is on a distinguished road
Quote:
Originally Posted by Nick View Post
This was indeed a problem, but I believe it is all fixed in the latest code. I've done a lot of tracking down of leaks with valgrind.
Seems like the new object piles fix most of the issues yes. See #3 for artifact structure copy in scramble_artifact()... this should still be a problem.
__________________
PWMAngband variant maintainer - check http://powerwyrm.monsite-orange.fr (or http://www.mangband.org/forum/viewforum.php?f=9) to learn more about this new variant!
PowerWyrm is offline   Reply With Quote
Old April 17, 2015, 09:22   #5
PowerWyrm
Prophet
 
PowerWyrm's Avatar
 
Join Date: Apr 2008
Posts: 2,672
PowerWyrm is on a distinguished road
Quote:
Originally Posted by PowerWyrm View Post
Seems like the new object piles fix most of the issues yes. See #3 for artifact structure copy in scramble_artifact()... this should still be a problem.
And there is... looking at artifact.spo, no brands/slays are ever generated on randarts.
__________________
PWMAngband variant maintainer - check http://powerwyrm.monsite-orange.fr (or http://www.mangband.org/forum/viewforum.php?f=9) to learn more about this new variant!
PowerWyrm is offline   Reply With Quote
Old April 17, 2015, 13:08   #6
Nick
Vanilla maintainer
 
Nick's Avatar
 
Join Date: Apr 2007
Location: Canberra, Australia
Age: 54
Posts: 7,862
Donated: $60
Nick will become famous soon enough
Quote:
Originally Posted by PowerWyrm View Post
And there is... looking at artifact.spo, no brands/slays are ever generated on randarts.
Yep, just found and fixed that one. I think it might fix a bunch of the randart problems.
__________________
One for the Dark Lord on his dark throne
In the Land of Mordor where the Shadows lie.
Nick is offline   Reply With Quote
Reply


Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)
 
Thread Tools
Display Modes

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off

Forum Jump

Similar Threads
Thread Thread Starter Forum Replies Last Post
found some great free music to play while playing angband Malak Darkhunter Idle chatter 0 March 28, 2015 15:59
3.5.0: Double free or corruption in SDL schuay Vanilla 6 January 6, 2014 14:53
Z+Angband and ASCII. Something massive in my head changed and I love it! Svladd Cjelik Variants 13 February 19, 2011 16:56
Pointers/Advice solicited on creating Angband solution inside VC++ 2008 Express jojojajumbo Development 5 August 5, 2009 04:21
[Un] Beginners pointers needed pndrev Variants 13 September 17, 2008 03:15


All times are GMT +1. The time now is 23:32.


Powered by vBulletin® Version 3.8.11
Copyright ©2000 - 2019, vBulletin Solutions Inc.