27 Dec, 2008, Korax wrote in the 1st comment:
Votes: 0
There is a stock memory leak in all versions of mindcloud in the function load_helps in db.c.

unfreed: <440> db.c(711), 48 bytes at 0x8b4ae0 {60 4B 8B 00 00 00 00 00 60 4A 8B 00 00 00 00 00 `K……`J……}

The line in question.
CREATE( pHelp, HELP_DATA, 1 );


The complete function.
/*
* Load a help section.
*/
void load_helps( FILE *fp )
{
HELP_DATA *pHelp;

for ( ; ; )
{
CREATE( pHelp, HELP_DATA, 1 );
pHelp->level = fread_number( fp );
pHelp->keyword = fread_string( fp );
if ( pHelp->keyword[0] == '$' )
break;
pHelp->text = fread_string( fp );
if ( pHelp->keyword[0] == '\0' )
{
STRFREE( pHelp->text );
STRFREE( pHelp->keyword );
DISPOSE( pHelp );
continue;
}

if ( !str_cmp( pHelp->keyword, "greeting" ) )
help_greeting = pHelp->text;
add_help( pHelp );
}
return;
}


I don't have much experience with memory management so I'd appreciate any help.
27 Dec, 2008, Omega wrote in the 2nd comment:
Votes: 0
is that on shutdown while in valgrind?
27 Dec, 2008, Korax wrote in the 3rd comment:
Votes: 0
That's on shutdown using memwatch. However I have run it through valgrind before and I get this.
==23801== 48 bytes in 1 blocks are definitely lost in loss record 4 of 28
==23801== at 0x4C1EB32: calloc (vg_replace_malloc.c:279)
==23801== by 0x4698A4: load_helps (db.c:710)
==23801== by 0x46C080: boot_db (db.c:432)
==23801== by 0x4653E6: main (comm.c:495)
27 Dec, 2008, Omega wrote in the 4th comment:
Votes: 0
on shutdown, are you clearing out the help_list which is being setup in add_help.
27 Dec, 2008, Korax wrote in the 5th comment:
Votes: 0
If you mean in my shutdown function then there is nothing that clears it. However when a program shuts down it would clear all the memory used regardless I would think.

void do_shutdown(CHAR_DATA * ch, char *argument)
{
extern bool merc_down;
CHAR_DATA *gch;
OBJ_DATA *obj;
char buf[MAX_STRING_LENGTH];
int i;

/*
* Store all mudwide data before doing anything else
*/
save_coreinfo();
save_kingdoms();

/*
* Make sure all chars are ready for the shutdown
*/
do_restore(ch, "all");
for (gch = char_list; gch != NULL; gch = gch->next)
{
if (IS_NPC(gch)) continue;

/* Fix any possibly head/object forms */
if (IS_HEAD(gch, LOST_HEAD) || IS_SET(gch->extra, EXTRA_OSWITCH))
{
REMOVE_BIT(gch->loc_hp[0], LOST_HEAD);
REMOVE_BIT(gch->affected_by, AFF_POLYMORPH);
REMOVE_BIT(gch->extra, EXTRA_OSWITCH);
gch->morph = str_dup("");
gch->pcdata->chobj = NULL;
gch->pcdata->obj_vnum = 0;
char_from_room(gch);
char_to_room(gch, get_room_index(ROOM_VNUM_ALTAR));
}

/* Take care of possibly artifacts */
for (i = 0; i < MAX_WEAR; i++)
{
if ((obj = get_eq_char(gch, i)) != NULL)
{
if (IS_SET(obj->quest, QUEST_ARTIFACT))
unequip_char(gch, obj);
}
}

/* Make sure the player is saved with all his eq */
do_stand(gch, "");
do_call(gch, "all");
save_char_obj(gch);
}

xprintf(buf, "Shutdown by %s.", ch->name);
append_file(ch, SHUTDOWN_FILE, buf);
xcatf(buf, "\n\r");
do_echo(ch, buf);
do_asave(ch, "changed");
do_forceauto(ch,"save");
do_autosave(ch,"");
merc_down = TRUE;
return;
}
27 Dec, 2008, Omega wrote in the 6th comment:
Votes: 0
I just looked at the source-code, mindcloud doesn't clear any of the existing lists on shutdown.

You'll have to clear out all the existing link-lists on shutdown for valgrind to be useful in finding memory leaks.

Things that will stand out if you've cleared all the links properly will be things like.

calling str_dup on a local variable within a function, and not clearing it. Thats where you'll find most of your memory related leaks/issue's, outside of that, if it is attached to a list, its normally not a leak :)
27 Dec, 2008, Omega wrote in the 7th comment:
Votes: 0
Essentaily, when you exit the app, memory is cleared anyways, everything that is associated with the app is usually terminated. But valgrind finds things that are still reachable when it is shutdown, its to detect to see if you've done double allocations, or if you've allocated something within a function and didn't clear it after it was used.

Essentially, if you clear all the lists, at shutdown, when the mud reaches exit(1); valgrind won't report anything other then *REAL* memory leaks. If you've done it right :)
27 Dec, 2008, Korax wrote in the 8th comment:
Votes: 0
How would I go about clearing the lists? For example pHelp.
if (pHelp)
free(pHelp);
27 Dec, 2008, Omega wrote in the 9th comment:
Votes: 0
I recommend you look at AFKmud, and take a gander at the cleanup functions (grep the source for it)

it is a perfect example of what to infact do.

I'll give an example of what should be done.

HELP_DATA *help, *help_next;
for(help = help_first; help; help = help_next) {
help_next = help->next;
free_help(help);
}


now if free_help doesn't exist, one would do this.
void free_help(HELP_DATA *help) {
STRFREE( help->text );
STRFREE( help->keyword );
DISPOSE( help );
}


Essentially, you'll have todo that for every list.

make a function, like this.

void cleanup_memory(void) {
HELP_DATA *help, *help_next;
for(help = help_first; help; help = help_next) {
help_next = help->next;
free_help(help);
}

return;
}


And toss in all your lists, like I did with help's, and clear everything out, go list-by-list, start with the small ones like helps until you get to know the code alittle better, and then slowly move up making more and more of these clearing routines. That will help you when it comes to valgrinding your code to get memory cleared.

You'd call cleanup_memory in your int main, right before the exit(1); at the bottom.

example
cleanup_memory();
exit(1);
27 Dec, 2008, Korax wrote in the 10th comment:
Votes: 0
That's very helpful I'll try to implement something like you've mentioned. I'll post back later if I get it working.

It seems there is a free_helps.

bool free_helps( CHAR_DATA *ch, char *arg )
{
char buf[MAX_STRING_LENGTH];
HELP_DATA *h, *h_next;
HELP_DATA *prev = NULL;
bool found = FALSE;

prev = help_first;
for ( h = help_first; h; h = h_next )
{
h_next = h->next;

if ( !str_cmp( h->keyword, arg ) )
{
free_string( h->keyword );
free_string( h->text );
if ( h == help_first )
help_first = h->next;
else
prev->next = h->next;
if ( h == help_last )
help_last = prev;
free_mem( h, sizeof(*h) );
found = TRUE;
}
prev = h;
}

if ( !found )
{
sprintf( buf, "Help entry %s not found.\n\r", arg );
send_to_char( buf, ch );
return FALSE;
}

return TRUE;
}
27 Dec, 2008, Omega wrote in the 11th comment:
Votes: 0
No problems, thats how I debug my mud for memory allocation issue's. It works nicely for me, and I know its done in AFKmud, and several other muds out there. Its important to understand the fundamentals of memory allocation when it comes to doing things like this though, if you do it wrong, you'll just cause it to crash.

In anycase, have fun with it, and do enjoy, its most worth it :)
27 Dec, 2008, Korax wrote in the 12th comment:
Votes: 0
Thanks again.
27 Dec, 2008, Guest wrote in the 13th comment:
Votes: 0
Korax said:
/*
* Load a help section.
*/
void load_helps( FILE *fp )
{
HELP_DATA *pHelp;

for ( ; ; )
{
CREATE( pHelp, HELP_DATA, 1 );
pHelp->level = fread_number( fp );
pHelp->keyword = fread_string( fp );
if ( pHelp->keyword[0] == '$' )
break;
pHelp->text = fread_string( fp );
if ( pHelp->keyword[0] == '\0' )
{
STRFREE( pHelp->text );
STRFREE( pHelp->keyword );
DISPOSE( pHelp );
continue;
}

if ( !str_cmp( pHelp->keyword, "greeting" ) )
help_greeting = pHelp->text;
add_help( pHelp );
}
return;
}


There's actually just a slight bit more to this one than not running free_helps() during the shutdown routine. Look at how load_helps() is setup. You see the condition there for what to do when it encounters the $ ? It breaks out of the infinite loop. What happened just prior to that detection and breakaway? pHelp->keyword was allocated - but it was not added to the list of helps. free_helps() walks the list, removing all the entries. Valgrind and memwatch are both correctly telling you that you've got one left somewhere that's not been handled. And it's the $ case that jumped out of the loop.

What I'd do in this case is take:
if ( pHelp->keyword[0] == '$' )
break;


and turn it into:
if ( pHelp->keyword[0] == '$' )
{
STRFREE( pHelp->keyword );
DISPOSE( pHelp );
break;
}


That way the rogue element that got left dangling is cleaned up properly.
28 Dec, 2008, Omega wrote in the 14th comment:
Votes: 0
good point :)
12 Jan, 2009, Korax wrote in the 15th comment:
Votes: 0
I thought I'd post a follow up. I tried what Samson suggested and the mud wouldn't boot after that. (it does load_helps on startup)

In the coming weeks I'll be trying to implement Jobo's double linked lists which should solve most of the issues relating to lists.

Snippet: http://www.dystopiamud.dk/snippets/list....
12 Jan, 2009, Guest wrote in the 16th comment:
Votes: 0
Strange. Does the MUD leave any logs behind as to why it won't boot?

/*
* Load a help section.
*/
void load_helps( FILE * fp )
{
HELP_DATA *pHelp;

for( ;; )
{
CREATE( pHelp, HELP_DATA, 1 );
pHelp->level = fread_number( fp );
pHelp->keyword = fread_string( fp );
if( pHelp->keyword[0] == '$' )
{
STRFREE( pHelp->keyword );
DISPOSE( pHelp );
break;
}
pHelp->text = fread_string( fp );
if( pHelp->keyword[0] == '\0' )
{
STRFREE( pHelp->text );
STRFREE( pHelp->keyword );
DISPOSE( pHelp );
continue;
}

if( !str_cmp( pHelp->keyword, "greeting" ) )
help_greeting = pHelp->text;
add_help( pHelp );
}
}


That's the same code as yours, pulled directly from SmaugFUSS. It's identical once you make the changes I suggested. It works perfectly in SmaugFUSS so I'm at a loss to explain why it would fail in Mindcloud.
12 Jan, 2009, Korax wrote in the 17th comment:
Votes: 0
Using the load_helps from your post.

Mon Jan 12 16:55:50 2009 :: Init data space.
Mon Jan 12 16:55:50 2009 :: loading help.are
*** glibc detected *** ../src/mcloud: free(): invalid pointer: 0x00002b91df044b2e ***
======= Backtrace: =========
/lib/libc.so.6[0x2b91ded22948]
/lib/libc.so.6(cfree+0x76)[0x2b91ded24a56]
../src/mcloud[0x46107a]
../src/mcloud[0x4613ee]
../src/mcloud[0x45a341]
/lib/libc.so.6(__libc_start_main+0xe6)[0x2b91deccd1a6]
../src/mcloud[0x4028b9]
======= Memory map: ========
00400000-00651000 r-xp 00000000 ca:01 158515 /home/kora/dev/src/mcloud
00851000-00853000 rw-p 00251000 ca:01 158515 /home/kora/dev/src/mcloud
00853000-008cb000 rw-p 00853000 00:00 0 [heap]
2b91ddb80000-2b91ddb9c000 r-xp 00000000 ca:01 639014 /lib/ld-2.7.so
2b91ddb9c000-2b91ddb9f000 rw-p 2b91ddb9c000 00:00 0
2b91ddd9b000-2b91ddd9d000 rw-p 0001b000 ca:01 639014 /lib/ld-2.7.so
2b91ddd9d000-2b91dddb3000 r-xp 00000000 ca:01 386978 /usr/lib/libz.so.1.2.3.3
2b91dddb3000-2b91ddfb3000 —p 00016000 ca:01 386978 /usr/lib/libz.so.1.2.3.3
2b91ddfb3000-2b91ddfb4000 rw-p 00016000 ca:01 386978 /usr/lib/libz.so.1.2.3.3
2b91ddfb4000-2b91ddfbc000 r-xp 00000000 ca:01 639009 /lib/libcrypt-2.7.so
2b91ddfbc000-2b91de1bc000 —p 00008000 ca:01 639009 /lib/libcrypt-2.7.so
2b91de1bc000-2b91de1be000 rw-p 00008000 ca:01 639009 /lib/libcrypt-2.7.so
2b91de1be000-2b91de1ec000 rw-p 2b91de1be000 00:00 0
2b91de1ec000-2b91de202000 r-xp 00000000 ca:01 639007 /lib/libpthread-2.7.so
2b91de202000-2b91de402000 —p 00016000 ca:01 639007 /lib/libpthread-2.7.so
2b91de402000-2b91de404000 rw-p 00016000 ca:01 639007 /lib/libpthread-2.7.so
2b91de404000-2b91de408000 rw-p 2b91de404000 00:00 0
2b91de408000-2b91de5c7000 r-xp 00000000 ca:01 389795 /usr/lib/libmysqlclient.so.15.0.0
2b91de5c7000-2b91de7c7000 —p 001bf000 ca:01 389795 /usr/lib/libmysqlclient.so.15.0.0
2b91de7c7000-2b91de812000 rw-p 001bf000 ca:01 389795 /usr/lib/libmysqlclient.so.15.0.0
2b91de812000-2b91de814000 rw-p 2b91de812000 00:00 0
2b91de814000-2b91de829000 r-xp 00000000 ca:01 639001 /lib/libnsl-2.7.so
2b91de829000-2b91dea28000 —p 00015000 ca:01 639001 /lib/libnsl-2.7.so
2b91dea28000-2b91dea2a000 rw-p 00014000 ca:01 639001 /lib/libnsl-2.7.so
2b91dea2a000-2b91dea2c000 rw-p 2b91dea2a000 00:00 0
2b91dea2c000-2b91deaae000 r-xp 00000000 ca:01 639010 /lib/libm-2.7.so
2b91deaae000-2b91decad000 —p 00082000 ca:01 639010 /lib/libm-2.7.so
2b91decad000-2b91decaf000 rw-p 00081000 ca:01 639010 /lib/libm-2.7.so
2b91decaf000-2b91dedf9000 r-xp 00000000 ca:01 639011 /lib/libc-2.7.so
2b91dedf9000-2b91deff8000 —p 0014a000 ca:01 639011 /lib/libc-2.7.so
2b91deff8000-2b91deffb000 r–p 00149000 ca:01 639011 /lib/libc-2.7.so
2b91deffb000-2b91deffd000 rw-p 0014c000 ca:01 639011 /lib/libc-2.7.so
2b91deffd000-2b91df207000 rw-p 2b91deffd000 00:00 0
2b91df20b000-2b91df221000 r-xp 00000000 ca:01 639039 /lib/libgcc_s.so.1
2b91df221000-2b91df421000 —p 00016000 ca:01 639039 /lib/libgcc_s.so.1
2b91df421000-2b91df422000 rw-p 00016000 ca:01 639039 /lib/libgcc_s.so.1
2b91e0000000-2b91e0021000 rw-p 2b91e0000000 00:00 0
2b91e0021000-2b91e4000000 —p 2b91e0021000 00:00 0
7fffccee0000-7fffccf2a000 rw-p 7fffccee0000 00:00 0 [stack]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0 [vdso]
13 Jan, 2009, Guest wrote in the 18th comment:
Votes: 0
Time to pop that into Valgrind and find out what it's so upset about. Doesn't make much sense to me unless fread_string behaves differently in Mindcloud.
13 Jan, 2009, Korax wrote in the 19th comment:
Votes: 0
It started up when I booted it with valgrind but I got the following in my valgrind log.

Mon Jan 12 20:36:58 2009 :: loading help.are
==15938== Invalid free() / delete / delete[]
==15938== at 0x4C2130F: free (vg_replace_malloc.c:323)
==15938== by 0x461079: load_helps (db.c:718)
==15938== by 0x4613ED: boot_db (db.c:436)
==15938== by 0x45A340: main (comm.c:489)
==15938== Address 0x60cb796 is 264,990 bytes inside a block of size 2,097,152 alloc'd
==15938== at 0x4C203E4: calloc (vg_replace_malloc.c:397)
==15938== by 0x46112B: boot_db (db.c:308)
==15938== by 0x45A340: main (comm.c:489)


db.c line 718

STRFREE( pHelp->keyword );


I'm guessing it's a double free.
0.0/19