jirentabu / crashrpt

Automatically exported from code.google.com/p/crashrpt
0 stars 0 forks source link

Memory Leak (or rather memory not deallocated before program exit) #169

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
***NOTE*** Please do create a NEW issue per each detected bug! Do not list
all detected problems in single issue record!

What steps will reproduce the problem?
1. Spawn lots of threads with shorts lifetime
2. Use the thread helper object

What is the expected output? What do you see instead?
Memory is properly released

What version of CrashRpt are you using?
1.31

What is your version of Visual Studio?
VS2010

What is your version of Windows operating system?
Win7 x64

Please provide any additional information below.

When lots of worker threads are spawned, the memory "leaks" go through the 
roof, even though crUninstallFromCurrentThread() is called.
Before the CrashRpt integration, the program was leak-free. Now a lot of 50 and 
34 byte long blocks are leaked.
I believe the issue is in CCrashHandler::UnSetThreadExceptionHandlers(), where 
g_sErrorMsg should be freed for the current thread ID. Otherwise spawning 
hundreds of short lived threads will blow up the map due to the different 
thread ID's.
I think one solution would be to include something alike to this:

g_cs.Lock();
int CCrashHandler::UnSetThreadExceptionHandlers()
    std::map<DWORD, CString>::iterator itMsg = 
        g_sErrorMsg.find(dwThreadId);

    if(itMsg==g_sErrorMsg.end())
    {
        g_sErrorMsg.erase(itMsg);
    }
g_cs.Unlock();

Original issue reported on code.google.com by joconner...@gmail.com on 3 Oct 2012 at 2:08

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by zexspect...@gmail.com on 3 Oct 2012 at 2:46

GoogleCodeExporter commented 9 years ago
Thanks for accepting the issue.
I have implemented the changes and now the leaks don't increase (I only have 
the ones from the Process handler, even though I do free that, too)
I noticed, too, though, that in crUninstall() the process-wide uninstaller is 
never called. Was that by design?

Either way, I have added a suggested fix below (see the .diff files, and the 
ori and modified ones are also attached).

Please let me know if you like it this way and will integrate it.

Original comment by joconner...@gmail.com on 3 Oct 2012 at 1:12

Attachments:

GoogleCodeExporter commented 9 years ago
Hi, thank you for the patch. I'll integrate it ASAP.

Original comment by zexspect...@gmail.com on 3 Oct 2012 at 2:46

GoogleCodeExporter commented 9 years ago
No problem.
I implemented these changes and they seem to work fine for me, but you should 
look over it to ensure it's not something stupid, since I only found CrashRpt 2 
days ago and am not entirely familiar with the internals yet.

Original comment by joconner...@gmail.com on 3 Oct 2012 at 7:24

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r1421.

Original comment by zexspect...@gmail.com on 8 Oct 2012 at 4:29