Zandronum Chat on our Discord Server Get the latest version: 3.1
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0002128Zandronum[All Projects] Bugpublic2015-03-19 00:372018-09-30 23:01
ReporterWatermelon 
Assigned ToDusk 
PrioritynormalSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformMicrosoftOSWindowsOS VersionXP/Vista/7
Product Version2.0 
Target Version2.0Fixed in Version2.0 
Summary0002128: Valgrind says there's a memory leak from P_OpenMapData [Server]
Description==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
Steps To ReproduceLoad 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.
Additional InformationI do not know if this affects the client (yet).
Attached Files

- Relationships

-  Notes
User avatar (0011860)
Watermelon (developer)
2015-03-19 00:47

I found where it's coming from, I'll bring a patch online shortly.
User avatar (0011861)
Watermelon (developer)
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).
User avatar (0011862)
Dusk (developer)
2015-03-19 14:09

Why not just use the sandbox like everybody else is doing..?
User avatar (0011863)
Watermelon (developer)
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)

User avatar (0011864)
Dusk (developer)
2015-03-19 18:55

Here is a pull request with a diff that is not absolute garbage.
User avatar (0011865)
cobalt (updater)
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(-)
User avatar (0011866)
Edward-san (developer)
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?

Issue Community Support
This issue is already marked as resolved.
If you feel that is not the case, please reopen it and explain why.
Supporters: No one explicitly supports this issue yet.
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2015-03-19 00:37 Watermelon New Issue
2015-03-19 00:42 Watermelon Description Updated View Revisions
2015-03-19 00:47 Watermelon Description Updated View Revisions
2015-03-19 00:47 Watermelon Assigned To => Watermelon
2015-03-19 00:47 Watermelon Status new => assigned
2015-03-19 00:47 Watermelon Note Added: 0011860
2015-03-19 02:37 Watermelon Note Added: 0011861
2015-03-19 14:09 Dusk Note Added: 0011862
2015-03-19 18:35 Watermelon Note Added: 0011863
2015-03-19 18:35 Watermelon Status assigned => needs review
2015-03-19 18:37 Watermelon Note Edited: 0011863 View Revisions
2015-03-19 18:55 Dusk Note Added: 0011864
2015-03-19 18:55 Dusk Assigned To Watermelon => Dusk
2015-03-19 19:16 cobalt Status needs review => needs testing
2015-03-19 19:16 cobalt Description Updated View Revisions
2015-03-19 19:16 cobalt Steps to Reproduce Updated View Revisions
2015-03-19 19:16 cobalt Note Added: 0011865
2015-03-20 01:08 Edward-san Note Added: 0011866
2015-03-29 20:20 Dusk Status needs testing => resolved
2015-03-29 20:20 Dusk Fixed in Version => 2.0
2015-03-29 20:20 Dusk Resolution open => fixed
2018-09-30 23:01 Blzut3 Status resolved => closed






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2024 MantisBT Team
Powered by Mantis Bugtracker