acr icon indicating copy to clipboard operation
acr copied to clipboard

113: Fix Valgrind problems

Open theonlypwner opened this issue 10 years ago • 2 comments

Fix Valgrind problems by d.ruler501 Issue 113 posted to Google Code on 2013 Apr 2 at 06:08:07

Current status: Accepted Current labels: Type-Defect, Priority-High

d.ruler501 on 2013 Apr 2 at 06:08:07:

What steps will reproduce the problem?

  1. Compile it with debugging enabled
  2. Run it with valgrind
  3. Play through the game with excruciatingly low frame rates(about 1/60 of what you'd usually have)
  4. Look at log

What is the expected output? What do you see instead? A mostly empty log with only system library problems. Instead I see a large amount of unitialized variable problems as well as memory leaks.

What version are you using? On what operating system? 2.5.7 trunk. Sabayon linux

Please provide any additional information below. Attached is the valgrind output from a singleplayer run with 4 bots. I'm not sure how much of the problems are important or not. The memory leaks that are shown at the end probably are though. I have also attached the output of assaultcube-reloaded so you can see if that gives any help.

val.log (641225 bytes, sha1: 1f43669b63d64154172e354e877dc59612d18baf) [reuploaded as val.log.txt]

stdout.log (1487 bytes, sha1: 5de57844d67201d5b558fe18d098020ed9cf1d16) [reuploaded as stdout.log.txt]

theonlypwner on 2013 Apr 3 at 23:07:44:

I looked at some parts of those logs.

I found that since players are equipped with weapons and those weapons are not deleted when the player leaves causes a memory leak.

Summary: Fix Valgrind problems (was: Valgrind shows all kinds of problems) Labels: -Priority-Unset Priority-High Status: Accepted

d.ruler501 on 2013 Apr 4 at 02:01:31:

Can you paste those lines here so we can make sure they are freed?

theonlypwner on 2013 Apr 4 at 23:04:52:

Most of those memory leak problems also exist in AssaultCube. If you download their game and run with valgrind, you should have the exact same errors, unless they are ACR-specific ones.

weapon.cpp void weapon::equipplayer(playerent *pl){ if(!pl) return; pl->weapons[WEAP_ASSAULT] = new m16(pl); ...

pl->weapons is never deleted (just like AC).

Some of these issues should be fixed.

d.ruler501 on 2013 Apr 10 at 05:11:47:

I'm attatching a new log for the server(on a 32 bit debian server). There is only 1 memory leak and I'll see if any of the other errors are important. As a note this was compiled with -03(AssaultCube Reloaded default) so line numbers are missing. I will fix this later.

serverleak.log (7198 bytes, sha1: e59ac2c79a68e3c51b7c5417f8c41f91d7b31892) [reuploaded as serverleak.log.txt]

theonlypwner on 2013 Apr 10 at 21:56:12:

I won't be able to fix that memory leak because I don't know where it is.

theonlypwner avatar Aug 09 '15 22:08 theonlypwner

ACR 2.5.7 was still based on AC 1.0.

We'll need new valgrind logs for ACR 2.6.3 because we were based on AC 1.2 starting in ACR 2.6.1.

theonlypwner avatar Jan 13 '16 01:01 theonlypwner

I used a similar tool from Microsoft, but it was not very useful:

  • a lot of memory is not explicitly cleaned up when ACR exits, but they aren't memory leaks, since we need them until the very end of the program (when they really should be cleaned up)
  • some allocations like newstring and hashtable in tools.h are used by different files, but Microsoft only provides one file location (after I edit the code to replace all the new allocations) unlike the trace in valgrind

However, I did find one leak: entposs in loadmapstats in tools.cpp (fixed in 6e48bf83e8203d530877d5041648acf8325ef859)

Since it will be difficult to find more leaks without refactoring to clean up memory at exit, I am removing the milestone target.

theonlypwner avatar Sep 01 '18 23:09 theonlypwner