MantisBT - Zandronum
View Issue Details
0002128Zandronum[All Projects] Bugpublic2015-03-19 00:372018-09-30 23:01
Watermelon 
Dusk 
normalmajoralways
closedfixed 
MicrosoftWindowsXP/Vista/7
2.0 
2.02.0 
0002128: Valgrind says there's a memory leak from P_OpenMapData [Server]
==29693== 7,168 bytes in 32 blocks are definitely lost in loss record 4,345 of 4,360
==29693== at 0x4C2B0E0: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==29693== by 0x73E6C7: P_OpenMapData(char const*) (p_setup.cpp:277)
==29693== by 0x68F928: NETWORK_Construct(unsigned short, bool) (network.cpp:464)
==29693== by 0x800F04: SERVER_Construct() (sv_main.cpp:395)
==29693== by 0x609E54: D_DoomMain() (d_main.cpp:2843)
==29693== by 0x596262: main (i_main.cpp:336)
==29693==
==29693== LEAK SUMMARY:
==29693== definitely lost: 7,168 bytes in 32 blocks
Load map01 from Doom2 with something like:

valgrind --log-file="valgrind.log" --leak-check=yes ./zandronum -host +sv_rconpassword test123

This was using the latest 2.0 build from the zandronum repository.
I do not know if this affects the client (yet).
No tags attached.
Issue History
2015-03-19 00:37WatermelonNew Issue
2015-03-19 00:42WatermelonDescription Updatedbug_revision_view_page.php?rev_id=6806#r6806
2015-03-19 00:47WatermelonDescription Updatedbug_revision_view_page.php?rev_id=6807#r6807
2015-03-19 00:47WatermelonAssigned To => Watermelon
2015-03-19 00:47WatermelonStatusnew => assigned
2015-03-19 00:47WatermelonNote Added: 0011860
2015-03-19 02:37WatermelonNote Added: 0011861
2015-03-19 14:09DuskNote Added: 0011862
2015-03-19 18:35WatermelonNote Added: 0011863
2015-03-19 18:35WatermelonStatusassigned => needs review
2015-03-19 18:37WatermelonNote Edited: 0011863bug_revision_view_page.php?bugnote_id=11863#r6809
2015-03-19 18:55DuskNote Added: 0011864
2015-03-19 18:55DuskAssigned ToWatermelon => Dusk
2015-03-19 19:16cobaltStatusneeds review => needs testing
2015-03-19 19:16cobaltDescription Updatedbug_revision_view_page.php?rev_id=6810#r6810
2015-03-19 19:16cobaltSteps to Reproduce Updatedbug_revision_view_page.php?rev_id=6812#r6812
2015-03-19 19:16cobaltNote Added: 0011865
2015-03-20 01:08Edward-sanNote Added: 0011866
2015-03-29 20:20DuskStatusneeds testing => resolved
2015-03-29 20:20DuskFixed in Version => 2.0
2015-03-29 20:20DuskResolutionopen => fixed
2018-09-30 23:01Blzut3Statusresolved => closed

Notes
(0011860)
Watermelon   
2015-03-19 00:47   
I found where it's coming from, I'll bring a patch online shortly.
(0011861)
Watermelon   
2015-03-19 02:37   
Patch made, I'll fork the main repo and apply it when I get the chance (and make my pull request).
(0011862)
Dusk   
2015-03-19 14:09   
Why not just use the sandbox like everybody else is doing..?
(0011863)
Watermelon   
2015-03-19 18:35   
(edited on: 2015-03-19 18:37)
Due to time constraints, I'll post this here and maybe someone else can do something:


diff -r a05d111285d8 src/network.cpp
--- a/src/network.cpp Wed Mar 18 20:53:03 2015 -0400
+++ b/src/network.cpp Wed Mar 18 20:53:36 2015 -0400
@@ -466,12 +466,20 @@
                 // [TP] The wad that had this map is no longer optional.
                 Wads.LumpIsMandatory( mdata->lumpnum );
             }
+
+ // [CK] If the map was opened, clean up the memory.
+ if ( mdata )
+ delete mdata;
         }
         catch ( CRecoverableError& e )
         {
             // [TP] Might as well warn the user now that we're here.
             Printf( "NETWORK_Construct: \\cGWARNING: Cannot open map %s: %s\n",
                 info.mapname, e.GetMessage() );
+
+ // [CK] If the map was opened, clean up the memory.
+ if ( mdata )
+ delete mdata;
         }
     }
 
@@ -1073,6 +1081,9 @@
             sum.AppendFormat ("%02X", BSum[j]);
 
         longSum += sum;
+
+ // [CK] If the map was opened, clean up the memory.
+ delete mdata;
     }
 
     CMD5Checksum::GetMD5( reinterpret_cast<const BYTE *>( longSum.GetChars( ) ),



(Edit this to copy the indentation)

(0011864)
Dusk   
2015-03-19 18:55   
Here is a pull request with a diff that is not absolute garbage.
(0011865)
cobalt   
2015-03-19 19:16   
Issue addressed by commit a23a45d032bf: - fixed memory leak in map collection checksum algorithm and optional wad processing (fixes 2128)
Committed by Teemu Piippo [Dusk] on Thursday 19 March 2015 20:51:01

Changes in files:
 src/network.cpp | 13 ++++++-------
 1 files changed, 6 insertions(+), 7 deletions(-)
(0011866)
Edward-san   
2015-03-20 01:08   
Just a curiosity: why there's no need to use the 'try-catch' structure around the 'P_OpenMapData' call in 'NETWORK_MapCollectionChecksum'?

Is it okay to assume that there should be no way for 'wadlevelinfos[].mapname' to return something corresponding to garbage data?