Link to home
Start Free TrialLog in
Avatar of mrwad99
mrwad99Flag for United Kingdom of Great Britain and Northern Ireland

asked on

Thread problem: causing obscene memory leaks !

Ah hello.

Right, I am creating a thread in my app that periodically checks the content of a dynamically alterable website.  The end result of all this is that the application should be able to check if it "authorised" to run or not; if it deems that it is not then it shuts itself down blah blah...

This is the code I create the thread with:

BOOL CGui4UnexApp::InitInstance()
{
               // usual stuff
      CWinThread* pThread = AfxBeginThread(ThreadFunc, NULL);
}

and this is the thread function

UINT CGui4UnexApp::ThreadFunc(LPVOID pParam)
{      
      CInternetSession session;
      CStdioFile* pFile = NULL;
      CString strLine;

      while (TRUE) {
            try {
                  pFile = session.OpenURL("http://www.yahoo.com", 0, INTERNET_FLAG_TRANSFER_BINARY
                                                      | INTERNET_FLAG_KEEP_CONNECTION);
                  pFile->ReadString(strLine);
                  TRACE("\n%s", strLine);
                  pFile->Close();
            } catch (CInternetException* e) {
                  e->Delete();
            }
            pFile->Close();
            session.Close();
            //Sleep(10000);
      }
      return 0;
}

All that the code does at the minute is get the first line of the Yahoo! web page source and output it.  Nothing special as yet.  However when running this code through the debugger I get loads of memory leaks, pertaining to strcore.cpp and inet.cpp, VC++ gives me constant first chance exception errors and I get the CInternetException error thrown all but the first time.  Notably, the exception does not get thrown if I move the variable declarations into the while loop....

These memory leaks do not occur when I do not run the thread.  Try it by plugging this code into a simple app wizard MDI app if you will.

I cannot see what these leaks or exceptions are coming from.  Can you ?

TIA
Avatar of jkr
jkr
Flag of Germany image

One thing that seems incorrect is that you are using a 'CStdioFile*' when 'OpenURL()' returns a 'CHttpFile*' (since the protocol is 'http://'...
Avatar of drichards
drichards

You're leaking at a minimum one thread object since you create a local pointer in InitInstance and can't delete it later.  Also, you are leaking a file object each time you successfully connect to the URL.  You need to delete pFile after use.  The wrong file type will work OK since CHttpFile derives from CStdioFile and the method returns a base class pointer.  CHttpFile just adds some methods specific to HTML files.

Not sure about the exception.  Are you allowed to reuse a CInternetSession?  You get a new one each time through if you move the 'session' variable inside the while block and you say it works OK then.
I checked the implementation of CInternetSession.  There is a session handle configured in the constructor and it is removed in the 'Close' method.  Once you close the session, you cannot reuse it.  Just don't close the session and it should work.
New code:
----------------------------------------
UINT CGui4UnexApp::ThreadFunc(LPVOID pParam)
{    
     CInternetSession session;
     CStdioFile* pFile = NULL;
     CString strLine;

     while (TRUE) {
          try {
               pFile = session.OpenURL("http://www.yahoo.com", 0, INTERNET_FLAG_TRANSFER_BINARY
                                             | INTERNET_FLAG_KEEP_CONNECTION);
               pFile->ReadString(strLine);
               TRACE("\n%s", strLine);
               pFile->Close();
               delete pFile;
          } catch (CInternetException* e) {
               e->Delete();
          }
          Sleep(10000);
     }
     return 0;
}
------------------------------------------
You'll still be leaking the CWinThread.  Not aesthetically pleasing, but also not a big deal since it runs until the app shuts down at which time the memory will be released anyway.  The file one is important because it was leaking every time through the while loop.
CWinThread object deletes itself when thread exits by default (see CWinThread::m_bAutoDelete).
Avatar of mrwad99

ASKER

OK. Thanks for all the replies.

jkr:  I was simply following this example - http://members.fortunecity.com/superjiang/vceb5/ch34j.htm
drichards: thanks for the new code and the research time you put in, but I still get the same memory leaks.
AlexFM: thanks for the thread note.

Please look at the following project, merely an appwizard MDI app with the thread code added:

http://mr_wad_99.europe.webmatrixhosting.net/FT.zip

I still get memory leaks pertaining to strcore.cpp just as a result of this thread !  

Why is this ?
I run the code without modification and get no leaks except the CWinThread.  I just let it run for a few seconds and then closed it.  Do you do something else?

I notice you are not using VS .NET 2003 as it asked to update the project files when I loaded it.
Avatar of mrwad99

ASKER

No, I also let it run for a few seconds and then close it.

This is what VC++ gives me in the debug window:

The thread 0xD64 has exited with code 0 (0x0).
The thread 0xDF0 has exited with code 0 (0x0).
Detected memory leaks!
Dumping objects ->
strcore.cpp(118) : {372} normal block at 0x00326E90, 2098 bytes long.
 Data: <        %   /   > 01 00 00 00 01 00 00 00 25 08 00 00 2F 00 CD CD
strcore.cpp(118) : {371} normal block at 0x00326618, 2098 bytes long.
 Data: <        %   www.> 01 00 00 00 0D 00 00 00 25 08 00 00 77 77 77 2E
strcore.cpp(118) : {193} normal block at 0x00326540, 141 bytes long.
 Data: <            <htm> 01 00 00 00 0C 00 00 00 80 00 00 00 3C 68 74 6D
plex.cpp(31) : {149} normal block at 0x00325E60, 124 bytes long.
 Data: <                > 00 00 00 00 00 00 00 00 04 00 CC 00 B8 FE A6 00
map_pp.cpp(72) : {148} normal block at 0x00326428, 68 bytes long.
 Data: <d^2             > 64 5E 32 00 00 00 00 00 00 00 00 00 00 00 00 00
thrdcore.cpp(166) : {132} client block at 0x00326068, subtype 0, 112 bytes long.
a CWinThread object at $00326068, 112 bytes long
Object dump complete.
The thread 0xCF8 has exited with code 0 (0x0).

What is happening here ?

<Points doubled>
Avatar of mrwad99

ASKER

Even more interestingly is that when I use the code at the link I posted above, (that does not use CStrings) I get no memory leak at all, not even the CWinThread !
In your thread function, you need to call AfxSocketInit().  Generally, AfxSocketInit() needs to be called once in each thread that uses sockets.

In addition, if you are linking statically and don't have SP4 or above, then look here: http://support.microsoft.com/default.aspx?scid=http://support.microsoft.com:80/support/kb/articles/Q193/1/01.ASP&NoWebContent=1

Best regards,
Mike
Avatar of mrwad99

ASKER

Mike,

I have tried that code and the memory leaks still persist.  I have SP5 for VC++ too.

I have posted a link again to this question; someone *must* know what is going on...

Cheers.
What you could try:

Add the following to a central header file:

#ifdef _DEBUG
#ifndef _DBG_NEW

#include <crtdbg.h>

inline void* __operator_new(size_t __n) {
     return ::operator new(__n,_NORMAL_BLOCK,__FILE__,__LINE__);
}
inline void* _cdecl operator new(size_t __n,const char* __fname,int __line) {
     return ::operator new(__n,_NORMAL_BLOCK,__fname,__line);
}
inline void _cdecl operator delete(void* __p,const char*,int) {
     ::operator delete(__p);
}

#define _DBG_NEW new(__FILE__,__LINE__)
#define new _DBG_NEW


#endif // _DBG_NEW
#else

#define __operator_new(__n) operator new(__n)

#endif

Then, add

               int tmpFlag;

               // Get the current state of the flag
               // and store it in a temporary variable
               tmpFlag = _CrtSetDbgFlag( _CRTDBG_REPORT_FLAG );

               // Turn On (OR) - Keep freed memory blocks in the
               // heap’s linked list and mark them as freed
               tmpFlag |= _CRTDBG_LEAK_CHECK_DF;

               // Set the new state for the flag
               _CrtSetDbgFlag( tmpFlag );


to 'InitInstance()' and

               _CrtDumpMemoryLeaks ();


to 'ExitInstance()' and you'll get the line number where the allocation occured.

(Ref.: http:Q_21009673.html)
Get an evaluation copy of BoundsChecker from NuMega/Compuware, you can get a 14 day trial license.

Use this to track down your leaks.

It's a great product, you might even wind up buying it because it is so useful.
Avatar of mrwad99

ASKER

wayside:

I looked at that and you have to be in some sort of business it would seem to get hold of it.  Now I do work but not in software engineering yet, so I don't think they would be interested in a mere 'student' getting their products.  But thanks anyway.

jkr:

I have done all of that, but had some difficulties.

Firstly I kept getting multiple warnings about multiple definitions of operator new and delete, so, after reading what you had to say on the referenced post, commented all of my

ifdef _DEBUG
#define new DEBUG_NEW
#undef THIS_FILE
static char THIS_FILE[] = __FILE__;
#endif

blocks out.  Then I kept getting

arning LNK4006: "void * __cdecl operator new(unsigned int,char const *,int)" (??2@YAPAXIPBDH@Z) already defined in (myfilename)

the same for operator delete too.  So after hunting around on the net I changed my build settings,

"...Choose "customize" in the drop down list and check "force output file".  Magically this will become warnings and not errors".

(Ref http://www.openh323.org/pipermail/openh323/2002-March/052965.html)

and get the memory leaks still but no hint as to where they are coming from !  All the source files indicated in the output of the debugger are not mine, eg strcore.cpp, sockcore.cpp so double clicking on them results in VC++ saying it cannot find the file.  No line numbers or anything useful to me.  What should I be getting ?

Here is the debugger's output (if that is helpful):

Dumping objects ->
strcore.cpp(118) : {410} normal block at 0x011E74C0, 2098 bytes long.
 Data: <        %   /   > 01 00 00 00 01 00 00 00 25 08 00 00 2F 00 CD CD
strcore.cpp(118) : {409} normal block at 0x011E8750, 2098 bytes long.
 Data: <        %   www.> 01 00 00 00 0D 00 00 00 25 08 00 00 77 77 77 2E
strcore.cpp(118) : {265} normal block at 0x011E6CE0, 141 bytes long.
 Data: <            <htm> 01 00 00 00 0C 00 00 00 80 00 00 00 3C 68 74 6D
sockcore.cpp(88) : {193} client block at 0x011E7468, subtype 0, 28 bytes long.
a CPtrList object at $011E7468, 28 bytes long
sockcore.cpp(86) : {192} client block at 0x011E7410, subtype 0, 28 bytes long.
a CMapPtrToPtr object at $011E7410, 28 bytes long
sockcore.cpp(84) : {191} client block at 0x011E73B8, subtype 0, 28 bytes long.
a CMapPtrToPtr object at $011E73B8, 28 bytes long
thrdcore.cpp(166) : {186} client block at 0x011E7090, subtype 0, 108 bytes long.
a CWinThread object at $011E7090, 108 bytes long
oleinit.cpp(86) : {138} client block at 0x011E68B0, subtype 0, 64 bytes long.
a CCmdTarget object at $011E68B0, 64 bytes long
plex.cpp(31) : {71} normal block at 0x011E4E40, 124 bytes long.
 Data: <             L  > 00 00 00 00 00 00 00 00 00 00 00 00 88 4C 1E 01
{70} client block at 0x011E4DD8, subtype 0, 32 bytes long.
a CDocManager object at $011E4DD8, 32 bytes long
strcore.cpp(118) : {69} normal block at 0x011E4D50, 63 bytes long.
 Data: <    2   2    Gui> 01 00 00 00 32 00 00 00 32 00 00 00 0A 47 75 69
C:\Documents and Settings\Administrator\My Documents\Study\Code\MFC\Gui4Unex - final version\Gui4Unex.cpp(138) : {68} client block at 0x011E4C88, subtype 0, 140 bytes long. <-----------------------------------***
a CMultiDocTemplate object at $011E4C88, 140 bytes long
strcore.cpp(118) : {67} normal block at 0x011E4BF0, 77 bytes long.
 Data: <    ?   @   http> 01 00 00 00 3F 00 00 00 40 00 00 00 68 74 74 70
strcore.cpp(118) : {66} normal block at 0x011E4B58, 79 bytes long.
 Data: <    A   B   http> 01 00 00 00 41 00 00 00 42 00 00 00 68 74 74 70
strcore.cpp(118) : {65} normal block at 0x011E4768, 25 bytes long.
 Data: <            Pane> 01 00 00 00 0B 00 00 00 0C 00 00 00 50 61 6E 65
strcore.cpp(118) : {64} normal block at 0x011E4AD0, 68 bytes long.
 Data: <    6   7   http> 01 00 00 00 36 00 00 00 37 00 00 00 68 74 74 70
strcore.cpp(118) : {63} normal block at 0x011E4A48, 68 bytes long.
 Data: <    7   7   http> 01 00 00 00 37 00 00 00 37 00 00 00 68 74 74 70
afxtempl.h(370) : {62} normal block at 0x011E49E0, 36 bytes long.
 Data: < F   F   G  TH  > 1C 46 1E 01 EC 46 1E 01 CC 47 1E 01 54 48 1E 01
strcore.cpp(118) : {61} normal block at 0x011E4958, 68 bytes long.
 Data: <    7   7   http> 01 00 00 00 37 00 00 00 37 00 00 00 68 74 74 70
strcore.cpp(118) : {60} normal block at 0x011E48D0, 71 bytes long.
 Data: <    :   :   http> 01 00 00 00 3A 00 00 00 3A 00 00 00 68 74 74 70
strcore.cpp(118) : {59} normal block at 0x011E4848, 65 bytes long.
 Data: <    4   4   http> 01 00 00 00 34 00 00 00 34 00 00 00 68 74 74 70
strcore.cpp(118) : {58} normal block at 0x011E47C0, 72 bytes long.
 Data: <    ;   ;   http> 01 00 00 00 3B 00 00 00 3B 00 00 00 68 74 74 70
strcore.cpp(118) : {56} normal block at 0x011E46E0, 75 bytes long.
 Data: <    >   >   http> 01 00 00 00 3E 00 00 00 3E 00 00 00 68 74 74 70
strcore.cpp(118) : {54} normal block at 0x011E4610, 65 bytes long.
 Data: <    4   4   http> 01 00 00 00 34 00 00 00 34 00 00 00 68 74 74 70
sockcore.cpp(88) : {51} client block at 0x011E4570, subtype 0, 28 bytes long.
a CPtrList object at $011E4570, 28 bytes long
sockcore.cpp(86) : {50} client block at 0x011E4518, subtype 0, 28 bytes long.
a CMapPtrToPtr object at $011E4518, 28 bytes long
sockcore.cpp(84) : {49} client block at 0x011E44C0, subtype 0, 28 bytes long.
a CMapPtrToPtr object at $011E44C0, 28 bytes long
{43} normal block at 0x011E4310, 28 bytes long.
 Data: < C   C          > 10 43 1E 01 10 43 1E 01 CD CD CD CD CD CD CD CD
plex.cpp(31) : {42} normal block at 0x011E2DD8, 124 bytes long.
 Data: <              U > 00 00 00 00 00 00 00 00 04 00 CC 00 B8 BC 55 00
map_pp.cpp(72) : {41} normal block at 0x011E2D50, 68 bytes long.
 Data: < -              > E8 2D 1E 01 00 00 00 00 00 00 00 00 00 00 00 00
Object dump complete.
Detected memory leaks!
Dumping objects ->
strcore.cpp(118) : {410} normal block at 0x011E74C0, 2098 bytes long.
 Data: <        %   /   > 01 00 00 00 01 00 00 00 25 08 00 00 2F 00 CD CD
strcore.cpp(118) : {409} normal block at 0x011E8750, 2098 bytes long.
 Data: <        %   www.> 01 00 00 00 0D 00 00 00 25 08 00 00 77 77 77 2E
strcore.cpp(118) : {265} normal block at 0x011E6CE0, 141 bytes long.
 Data: <            <htm> 01 00 00 00 0C 00 00 00 80 00 00 00 3C 68 74 6D
thrdcore.cpp(166) : {186} client block at 0x011E7090, subtype 0, 108 bytes long.
a CWinThread object at $011E7090, 108 bytes long
plex.cpp(31) : {42} normal block at 0x011E2DD8, 124 bytes long.
 Data: <     -        U > 00 00 00 00 F4 2D 1E 01 04 00 CC 00 B8 BC 55 00
map_pp.cpp(72) : {41} normal block at 0x011E2D50, 68 bytes long.
 Data: < -              > E8 2D 1E 01 00 00 00 00 00 00 00 00 00 00 00 00
Object dump complete.
The thread 0x8BC has exited with code 0 (0x0).
The thread 0x6B0 has exited with code 0 (0x0).
The thread 0x324 has exited with code 0 (0x0).
The thread 0x910 has exited with code 0 (0x0).
The thread 0x814 has exited with code 0 (0x0).
The thread 0x860 has exited with code 0 (0x0).
The thread 0x500 has exited with code 0 (0x0)

Ah, after reading that again you can see one line that is mine (as indicated) but that is this:

      CMultiDocTemplate* pDocTemplate;
      pDocTemplate = new CMultiDocTemplate(
            IDR_GUI4UNTYPE,
            RUNTIME_CLASS(CGui4UnexDoc),
            RUNTIME_CLASS(CChildFrame), // custom MDI child frame
            RUNTIME_CLASS(CGui4UnexView));
      AddDocTemplate(pDocTemplate);

and that has never caused a leak before.

What on earth is going on with this thread ?????
The sockcore problem is in AfxSocketinit.  Are you calling this?  Since you are not using MFC sockets, I would not call AfxSocketInit (unless you ARE using MFC socket class somewhere?).  This is the source of the CMapPtrToPtr and CPtrList leaks.  These result in secondary leaks of the bytes allocated by the CMap... and CPtrList objects.

You then have the problem with the CMultiDocTemplate.  I'd check if you are creating multiple templates and look at how you exit the app to see what might be wrong here.  Or maybe how did you create the project?  It looks almost as if some compiler settings are incorrect for an MFC app or something.

FYI, the source files listed in your debugger output are in the MFC source directory under your Visual Studio installation - VC98/MFC/src - if you installed source with VS.  Otherwise, you won't have these files.
ASKER CERTIFIED SOLUTION
Avatar of MikeAThon
MikeAThon

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of mrwad99

ASKER

Right.

drichards:

I was calling AfxSockInit() but that has now been removed and the sockcore leaks appear to have gone away.  Regarding the CMultiDocTemplate, then there is nothing wrong with neither my code nor the compiler settings as I have tested this by creating a bog standard MDI app (indeed the one I linked to in an earlier post), just adding my thread code as listed.

MikeAThon:

I initially suspected something like that, but I then proceeded to remove the sleep() and the while loop to ensure the thread only ran once, and still got errors.  I tried the code you suggested and still had the leaks.

I have thus decided to go back to basics with this.  THe memory leaks are *without a shadow of a doubt all to do with the Internet functions*.  Take this for example, from the project I linked to above:

// Incredibly simply thread function
UINT CFTApp::ThreadFunc(LPVOID pParam)
{
     CInternetSession session;

     while (TRUE) {

          } catch (CInternetException* e) {
               e->Delete();
          }
          Sleep(1000);
     }
      
     return 0;

}
Gives the following memory leaks:

plex.cpp(31) : {144} normal block at 0x002F6630, 124 bytes long.
 Data: <                > 00 00 00 00 00 00 00 00 04 00 CC 00 B8 FE 93 00
map_pp.cpp(72) : {143} normal block at 0x002F65A8, 68 bytes long.
 Data: <4f/             > 34 66 2F 00 00 00 00 00 00 00 00 00 00 00 00 00
thrdcore.cpp(166) : {131} client block at 0x002F6270, subtype 0, 112 bytes long.
a CWinThread object at $002F6270, 112 bytes long
Object dump complete.

Comment out the

CInternetSession session;

however and the first two leaks go away.  Regarding the last CWinThread leak:

      CWinThread* pThread;
      pThread = AfxBeginThread(ThreadFunc, (LPVOID) this, THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
      pThread->m_bAutoDelete = TRUE;
      pThread->ResumeThread();

declares and sets it up, but why the heck is it still leaking when I explicitly set m_bAutodelete to TRUE ?!  Given I could get around this by making it a member variable and deleting it OnExitInstance, but it still should not behave like this.

I am going to read up seriously on the Internet functions, but in the meantime I would appreciate any ideas on why they appear to cause such bizzare leaks.

Many thanks for the help and advice so far.
Avatar of mrwad99

ASKER

Right am back now, was away from my machine for a bit.

The main problem is the TRUE loop in the thread function.  Without it, no memory leaks, but with it, well, it all goes wrong.

Does anyone know what is going on here ?  Or am I going to have to close this without getting a definite answer - that would be the first time on EE *ever*...
<snip, from OPer>...The main problem is the TRUE loop in the thread function.  Without it, no memory leaks, but with it, well, it all goes wrong.... </snip>


You're giving us inconsistent information now.  You previously told us that even with the TRUE loop, the main leaks go away if you delete CInternetSession:

<snip, from OPer>...THe memory leaks are *without a shadow of a doubt all to do with the Internet functions...</snip>

Slow down a bit, step back, and re-group.  There have been a lot of approaches tried by you so far, and maybe it might help if you post code showing your current approach.  Post your actual code, not pseudo-code (one sample you posted above had a catch without a try, so it was probalby pseudo-code).

Better, is there a minimal test application that you can write that also shows the problem?  Post that code too.

Mike
The leak is from _afxSessionMap, defined as CSessionMapPtrToPtr, which is used by CInternetSession and CHttpFile (and other things not present in your code).  I will try VC6 and see if I get the leak.  VC7 does not exhibit this leak.

>> proceeded to remove the sleep() and the while loop to ensure the thread only ran once, and still got errors

>> problem is the TRUE loop in the thread function.  Without it, no memory leaks

Which is correct?
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
I had suggested something along these lines (i.e., the problem lies in failure to shut down the thread gracefully), in my post above at https://www.experts-exchange.com/questions/21091944/Thread-problem-causing-obscene-memory-leaks.html#11856282 

But the OP said that he "tried the code you [meaning me] suggested and still had the leaks. "
>> I had suggested something along these lines
Yep, I noticed that.  Just thought I'd try it myself and verify results since we seemed to be getting conflicting data.

And I left a line out of the code in #2 of my post.  At the bottom of the 'while (true)' loop, I put 'if (WaitForSingleObject(hEvent, 5000) == WAIT_OBJECT_0) break;' and removed the 'Sleep'.  This waits 5 seconds between loop iterations and exits the loop if you set the event.
That's a clever way of avoiding my (more clumsy) waitable timer.

Regards,
Mike
Avatar of mrwad99

ASKER

People,

just a quick update to say that now I have re-done my application and added the code suggested above by MikaAThon I am no longer getting the threads.  I am just going to read up on exactly what has been done, as there is a lot of new code here that I am still not sure how works, then I am going to look at that code provided by drichards before finally closing this question.

Many thanks, will be back very soon.
If you try my code, be sure to pick up the one omission that I pointed out in my last post.  Otherwise, the thread will not exit.
Avatar of mrwad99

ASKER

Righty-O.

I have had a look at that code in depth and have not been able to find much info on what you have done from the usual sources (i.e. MSDN) that does not *just* explain what the functions do; i.e.  without giving them in an understandable example.  So...

Anyone who can solve this please, not necessarily just the author:

>>       HANDLE hEvents[2];

>>          hTimer = CreateWaitableTimer(NULL, FALSE, NULL);
>>          SetWaitableTimer(hTimer, &liDueTime, 1000, NULL, NULL, 0);
      
>>      hEvents[0] = hTimer;
>>      hEvents[1] = pApp->m_hEvent;

Okey dokey.  I get that we are creating a timer, and then setting it to run every one second (1000 milliseconds).  But what I don't get is the need for the two HANDLE objects, especially the Event objec that is initialised in InitInstance():

>>      m_hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);

Then, in my work loop:

>>      while (pApp->bKeepRunning) {
>>            // Do my stuff...
>>
>>            WaitForMultipleObjects(2, hEvents, FALSE, INFINITE);
>>            
>>      }
>>      CloseHandle( hTimer);

Hmm.  I guess that we do the stuff in the loop, then wait for the timer to become signalled (which happens after one second) then wait for the event object too <-- but why ?  What is the event doing ?  Why don't we just wait for the timer and be done ?

Then, in ExitInstance():

>>      SetEvent( m_hEvent);
>>      WaitForSingleObject( m_pThread->m_hThread, INFINITE);
>>      CloseHandle(m_hEvent);

What the heck is going on here, and why ?  Why don't we just wait for the thread full stop, without the other two lines ?

Sorry about this, but I just don't understand that and at my level of experience that is understandable (I hope).

TIA
Go back and re-read the numbered items 1 through 5 in my post.  It explains what's happening.

You need two HANDLEs in the thread because you want your loop to run on either of two events: expiration of the timer and exit from the application.  So, in ExitInstance, you set the bKeepRunning BOOL to FALSE and then signal the m_hEvent event, which causes the thread to re-run its loop, discover that the bKeepRunning BOOL is now FALSE, and exit cleanly.

But all that won't happen until the thread gets its next timeslice, and meanwhile your main app's thread would otherwise continue to run and cause the problems you're seeing.  So, in ExitInstance, after signalling m_hEvent, you WaitForSingleObject on the thread's handle.  This causes the main app's thread to pause until the thread's handle signals, which happens automatically when the thread exits.  As soon as the main app sees this, it knows the thread has exited safely, and that it's OK for the main app to shut down too.

There may be some confusion over the way the timer and the wait are implemented in that code.  With only the line:

  WaitForMultipleObjects(2, hEvents, FALSE, INFINITE);

the thread function will wait for the event or the timer and just keep looping.  The code should be (I think this was the intent):

    // This could be simplified by putting the event handle first and the timer second in the array...
    if (WaitForMultipleObjects(2, hEvents, FALSE, INFINITE) == (WAIT_OBJECT_0+1)) break;

>> Why don't we just wait for the timer and be done ?

Because the timer is just controlling the frequency of the loop.  The event is what is signalling the thread that the dialog is closing and the thread should terminate itself.  When the event is signalled, the thread function breaks out of the loop and exits.  The timer is not really necessary as you can do:

    if (WaitForSingleObject(pApp->m_hEvent, 1000) == WAIT_OBJECT_0) break;

to achieve the same result.  That's the only difference between my code and MikeAThon's.

>> What the heck is going on here, and why ?

This is where the event is signalled to terminate the thread.  Without this, the thread just loops forever and does not exit.  After signalling the event, you need to wait for the thread to actually exit or you haven't gained anything since that was the original problem.  Thus, you have to wait on the thread handle.  Once complete, you need to clean up the event handle.  So thoise are the last threee lines:  set event, wait for the thread, and clean up the event handle.
Avatar of mrwad99

ASKER

Right then.

I have exhaustively read up on all the concepts that have been mentioned, as well as modifiying it to see what happens if...and all that.  I have realised that:

the Event object and the Timer are not *strictly* necessary; they are just there to improve the way that the thread works.  By this I mean that all that is required in order to prevent the memory leak is to simply wait for the thread to exit in ExitInstance() - I could simply add the Sleep(10000) to the thread and be done.  However, doing this of course means that if the thread is just entering the ten second sleep (or what would more realistically be a longer time period) then the application as a whole will not exit until this period is over.  

With the timer and event object, this is of course not an issue as has been explained.

I then tried the code given by drichards and realised that is just the same principally except it is done with fewer lines of code.

Overall you two have taught me a heck of a lot about things that I did not know (well, I have a big book on Win32 threading but it never sinks in until you actually apply it) and I really do appreciate it.  

I am hence going to split the 500 points on offer equally between you both.

Many thanks once again, it really is appreciated :)
You should free memory with delete pFile after closing your file.
Avatar of mrwad99

ASKER

Yeah I do do cheers.