28 Dec, 2007, Kline wrote in the 1st comment:
Votes: 0
Ok, so off the top, I'm horrible with memory management :( It's one of the few things that hasn't quite clicked for me as far as what I know about C. Lately I seem to be having a constant, slow leak, and I think it is causing possible corruption. I've ran Valgrind quite a few times to the point of ripping my hair out and fixed a few small (but unrelated) problems.

My problem is with alloc_perm() and the block lists it uses. I don't believe the lists for data like affects are being utilized properly or such.

void affect_remove( CHAR_DATA *ch, AFFECT_DATA *paf)
{
if ( ch->affected == NULL )
{
bug( "Affect_remove: no affect.", 0 );
return;
}
affect_modify( ch, paf, FALSE);

if ( paf == ch->affected )
{
ch->affected = paf->next;
}
else
{
AFFECT_DATA *prev;

for ( prev = ch->affected; prev != NULL; prev = prev->next )
{
if ( prev->next == paf )
{
prev->next = paf->next;
break;
}
}
if ( prev == NULL )
{
bug( "Affect_remove: cannot find paf.", 0 );
return;
}
}
paf->next = affect_free;
affect_free = paf->next;
return;
}


If I cast a spell on myself, say, fly, then check memory stats I notice my perm block usage has gone up by 1 and a few bytes of total memory. Casting a second spell, sanctuary, yields the same results. However, if I use dispel magic which just recursively calls affect_remove(), perm block usage either INCREASES or stays the same. However if it stays the same, I should be able to re-cast the same spells and it should use already alloc'ed memory, correct? But each time it re-allocs more.

After the game has been running for a day or so, and memory usage is upwards of 20mb, sometimes characters seem to corrupt. Most notably people's ch->home for recall, is an int. It will randomly change to some value between 1 and 10. I've checked every place in the code that references that, and nowhere explicitly sets it except to initialize it in load_char_obj(), read it in do_recall(), or change if a player chooses to with do_home(). Values in some people's ch->pcdata structs are sometimes changing too, and I can't ever reproduce it. A value is there then suddenly it is zeroed out.

I really don't know if I described that very well as it's easy for me to what what happens in my game but hard to describe it =\.
This is based on a flavor of God Wars, but I believe all the memory management is still the same as stock Merc.

Any help, or even a nudge in the right direction, would be appreciated! I run this all on my own in-home Debian box, so any different debugging tools, libs, etc, are no problem to get. I just want to try to find a resolution :(
28 Dec, 2007, Davion wrote in the 2nd comment:
Votes: 0
Well after affect_remove() is called, the mem should stay the same, as the memory management never actually frees anything. Your problem might be with adding the affect, it may simply allocate instead of grabbing it from the affect_free list. This would also not show up under valgrind because the sector isn't lost, as there's a pointer to it somewhere in the affect_free list.
28 Dec, 2007, Kline wrote in the 3rd comment:
Votes: 0
Davion said:
Well after affect_remove() is called, the mem should stay the same, as the memory management never actually frees anything.
And I see this, but it doesn't seem to be the case as memory usage will sometimes go up :(

Davion said:
Your problem might be with adding the affect, it may simply allocate instead of grabbing it from the affect_free list.
Could be, but it is currently set to check for a free one before alloc() a new one, so I'm not sure why it isn't.

void affect_to_char( CHAR_DATA *ch, AFFECT_DATA *paf )
{
AFFECT_DATA *paf_new;

if ( affect_free == NULL )
{
paf_new = alloc_perm( sizeof(*paf_new) );
}
else
{
paf_new = affect_free;
affect_free = affect_free->next;
}
*paf_new = *paf;
paf_new->next = ch->affected;
ch->affected = paf_new;

affect_modify( ch, paf_new, TRUE);
return;
}
28 Dec, 2007, Omega wrote in the 4th comment:
Votes: 0
setup your mud to clear all memory lists upon shutdown. freeing the actual data, not just adding them to a free list.

once this is done, run valgrind with the show-reachable option, run the mud in valgrind for a good while, (my suggestion is a day) if it doesn't crash in that time, shut down the mud, and once the mud clears all the lists, you'll be left with the reachable memory, IE, anything that has leaked out, won't get free'd, therefor, your leak will become apparent. But be warned, using rom, your bound to find more then just 1 memory leak with the initial memory system, hell, you'll be lucky if you find under 30.

thats my suggestion, it should help you find any leak your mud has. As they won't get cleared with the list clearing.
28 Dec, 2007, Davion wrote in the 5th comment:
Votes: 0
Hmm. Mind posting the code to one of the offending spells?
28 Dec, 2007, Kline wrote in the 6th comment:
Votes: 0
Darien said:
setup your mud to clear all memory lists upon shutdown. freeing the actual data, not just adding them to a free list.

once this is done, run valgrind with the show-reachable option, run the mud in valgrind for a good while, (my suggestion is a day) if it doesn't crash in that time, shut down the mud, and once the mud clears all the lists, you'll be left with the reachable memory, IE, anything that has leaked out, won't get free'd, therefor, your leak will become apparent. But be warned, using rom, your bound to find more then just 1 memory leak with the initial memory system, hell, you'll be lucky if you find under 30.

thats my suggestion, it should help you find any leak your mud has. As they won't get cleared with the list clearing.
Thanks for this, I'll get working on it now :) Valgrind already bitches at me for some of the MySQL code not freeing itself on exit, so guess it's not a bad time to clean everything up.

Davion said:
Hmm. Mind posting the code to one of the offending spells?
Certainly, here's a rather simple one. They all use the same method; just different bits and messages and such.

void spell_fly( int sn, int level, CHAR_DATA *ch, void *vo )
{
CHAR_DATA *victim = (CHAR_DATA *) vo;
AFFECT_DATA af;

if ( IS_AFFECTED(victim, AFF_FLYING) )
return;
af.type = sn;
af.duration = level + 3;
af.location = 0;
af.modifier = 0;
af.bitvector = AFF_FLYING;
affect_to_char( victim, &af );
send_to_char( "You rise up off the ground.\n\r", victim );
act( "$n rises up off the ground.", victim, NULL, NULL, TO_ROOM );
return;
}
30 Dec, 2007, Omega wrote in the 7th comment:
Votes: 0
I always hated the affect system. I wrote up a function that dropped/modified affects on a character. but its useless since i completely redid the system to make it work :P HAHA :P
30 Dec, 2007, David Haley wrote in the 8th comment:
Votes: 0
I've always hated the fact that the SMAUG "affect" system is responsible for so many misspellings of affect vs. effect. :tongue:
30 Dec, 2007, Kayle wrote in the 9th comment:
Votes: 0
not to further confuse anyone reading, but are you saying they should be called effects?

Because technically, while the spell is causing an effect on the character the spell is affecting the character so it would make perfect sense for the spells to create affects that are then stored on the character, because while it is an effect on the character it is a state of being affected by the spell being cast, savvy?
30 Dec, 2007, David Haley wrote in the 10th comment:
Votes: 0
Of course it should be spelled 'effect' if it is being used as a noun. And of course it should be spelled 'affect' if it is being used as a verb (or a participle as in 'affected'). When you write "spells to create affects" you are not saying what you think you're saying; 'affect' as a noun doesn't mean the same thing at all.
31 Dec, 2007, Guest wrote in the 11th comment:
Votes: 0
While I understand the reasons for why Rom's memory management is the way it is, I don't like it at all. It's far too confusing and from what I've played with it's also very difficult to diagnose problems like this. I've always preferred the way Smaug does it. Allocate something when you need it. Destroy it when you're done. No silly free_list things to keep track of.
31 Dec, 2007, Davion wrote in the 12th comment:
Votes: 0
Samson said:
While I understand the reasons for why Rom's memory management is the way it is, I don't like it at all. It's far too confusing and from what I've played with it's also very difficult to diagnose problems like this. I've always preferred the way Smaug does it. Allocate something when you need it. Destroy it when you're done. No silly free_list things to keep track of.


He's using godwars, so I don't understand why Rom's MM would be an issue… As for Kline… all the code looks sound. Are you sure that's where the memory's spiking? Try stepping through the casting process with gdb to see exactly what memory is being bumped.
31 Dec, 2007, Guest wrote in the 13th comment:
Votes: 0
Sorry. It looked more or less like the same system to me. It's probably all common to Merc anyway.

Either way, I never have liked the way that's done.
31 Dec, 2007, Kline wrote in the 14th comment:
Votes: 0
Davion said:
He's using godwars, so I don't understand why Rom's MM would be an issue… As for Kline… all the code looks sound. Are you sure that's where the memory's spiking? Try stepping through the casting process with gdb to see exactly what memory is being bumped.


It was spiking in all sorts of places…Allocating new obj data for corpses, new affect data for spells…everyplace. I could place an affect on myself, see it alloc one, dispel it, place a new affect, and see it not re-use the already allocated memory.

I never have found a decent way to track any of the things it does with the stock memory management, so I just recently re-routed them to actual calloc() calls earlier tonight. So far memory usage is much lower than previously (I imagine because everything is only allocating the size it needs, not a fixed block size) and I haven't -yet- noticed a steady leak. Still giving it a test run with players on now, though :)
31 Dec, 2007, David Haley wrote in the 15th comment:
Votes: 0
Samson said:
Allocate something when you need it. Destroy it when you're done. No silly free_list things to keep track of.

This is sometimes more easily said than done. The problem is when you aren't sure who owns a pointer, and you need to nullify every single reference to an object before destroying it. Not actually deleting something but keeping it on a list for a while can make that task easier since a dereference of the pointer will not cause a crash; instead, you'll dereference something that should be gone and something a little odd might happen (like trying to attack a mob that is not really there), but more importantly you won't crash.

Of course, these issues should be solved by having some kind of memory management scheme where this stuff is dealt with automatically. It's easier in C++ to implement, though, because you can overload the assignment operators: you can enforce the memory policy, rather than hope that people will follow policy.

Incidentally, my copy of SMAUG has some of those free list thingies lying around, although that could be a vestige of it being from SMAUG 1.0 originally…
31 Dec, 2007, Kline wrote in the 16th comment:
Votes: 0
I actually started the undertaking of removing free_lists and just keeping things in a list, simply for when I need to iterate through them all. Not using any of the stock Diku/Merc MM code past a few of the free_lists at this point. Just added some wrappers to call calloc() and free() where needed so I could track statistics.

Things are getting a little trickier getting into shared things like the char_list that are used between mobs and PC's, but I'll figure those out soon after I get to dig around the code a bit more :)

edit: Somehow what is being reported through my own tracking and from mallinfo() still varies by about 160kb…odd. Any possible reasons for this?
edit edit: nm, stupid CREATE() macros I forgot about. :)
edit edit edit: Wow, so that wasn't it, still getting different readings from my own counting and mallinfo() :P
04 Jan, 2008, Omega wrote in the 17th comment:
Votes: 0
by removing free_lists, and replacing with calloc/free is a good way to go, HOWEVER, there is a small issue with that statement, if your dealing with rom, you'll want to modify your raw_kill location, as after raw_kill, it process's the autoloot and such, and in anycase, searching names and such causes issue's, i recommend doing some serious work before you do that.

With that said, I recommend keeping free_lists, but at the end of the update_handler function, have them clear, that way, they are purged from the memory, without risk of losing the actual data before hand (which is the only time it can seemingly be used) I encountered these issue's before, thus why I make the recommendation. Save you some time in the long run.
05 Jan, 2008, Kline wrote in the 18th comment:
Votes: 0
Not sure where it got started that I am using ROM, as its God Wars, but thanks none the less :D

I'm on a business trip ATM so I only removed the affect_free list to see how things would go – affects are widely used but fairly simple, not like char_list sharing different data structs between PC/NPC.

I'm still having very strange errors though, that I can't find any other blame point for besides the char_free list. People's recalls randomly change. Some people have powers randomly change values. Other different pfile values randomly change. Nothing is ever consistent and I can't seem to reproduce ANY of it myself. Could this possibly -at all- be bad RAM? I run my game on a home server that I bought as an Outpost.com $200 special. I just tossed a gig of bargin bin RAM into it 2 years ago up from the 128 it had, but I think I have some quality Kingston sitting around I could try swapping if anybody thinks my odd problems could be RAM related.
05 Jan, 2008, Omega wrote in the 19th comment:
Votes: 0
the question you should ask, are people's recall's randomly changing in-game, or only after they log-in, because if it is only after log-in, then it would be best to look at the loading of pfiles to see if they are loading correctly, and/or check to see if when they are being pulled from the free_list that they have been properly cleared, because if they haven't been cleared properly, when they load, and the load doesn't work, they'll contain the last-users recall point.

this is why I hate using the free_lists. Oi.
05 Jan, 2008, Kline wrote in the 20th comment:
Votes: 0
It seems to be in-game. The thing with the recalls, is that they all set themselves to the same range of numbers (0-7 I believe, most commonly 3). Once I manage to entirely rid myself of free_lists I suppose I'll know for certain if this is the issue or if it is something else. Everything is getting initialized to a zero, empty string, or NULL in the load_char(). So just somewhere randomly during gameplay people have data changing for no reason.
0.0/31