Solved

Thread problem: causing obscene memory leaks !

Posted on 2004-08-12
32
2,100 Views
Last Modified: 2013-11-20
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
0
Comment
Question by:mrwad99
  • 11
  • 10
  • 6
  • +4
32 Comments
 
LVL 86

Expert Comment

by:jkr
ID: 11787697
One thing that seems incorrect is that you are using a 'CStdioFile*' when 'OpenURL()' returns a 'CHttpFile*' (since the protocol is 'http://'...
0
 
LVL 19

Expert Comment

by:drichards
ID: 11787890
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.
0
 
LVL 19

Expert Comment

by:drichards
ID: 11787972
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.
0
 
LVL 19

Expert Comment

by:drichards
ID: 11788012
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.
0
 
LVL 48

Expert Comment

by:AlexFM
ID: 11791414
CWinThread object deletes itself when thread exits by default (see CWinThread::m_bAutoDelete).
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11792339
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 ?
0
 
LVL 19

Expert Comment

by:drichards
ID: 11792907
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.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11795705
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>
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11795926
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 !
0
 
LVL 2

Expert Comment

by:MikeAThon
ID: 11826257
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
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11835099
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.
0
 
LVL 86

Expert Comment

by:jkr
ID: 11835376
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)
0
 
LVL 14

Expert Comment

by:wayside
ID: 11844074
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.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11854338
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 ?????
0
 
LVL 19

Expert Comment

by:drichards
ID: 11855579
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.
0
 
LVL 2

Accepted Solution

by:
MikeAThon earned 250 total points
ID: 11856282
OK, maybe it's simply a problem with your thread not shutting down gracefully.  I just noticed, for example, that one main difference between your code and the code that works fine is that you loop while (TRUE) and Sleep 10 seconds in the loop.  Maybe there's a problem with termination of a thread that's in the middle of a 10 second Sleep.

Here's what I would try.

1. Change the local stack variable pThread (from CGui4UnexApp::InitInstance) into a member variable.  You'll need it later.  

2. Add a BOOL bKeepRunning member variable to your CGui4UnexApp class.  The thread would test this flag before  continuing in its loop.  To give the thread access to it, pass the "this" pointer as the second parameter in AfxBeginThread, and then (in ThreadFunc) cast the pParam variable back into a CGui4UnexApp pointer and store it in a local variable; then you can access all member variables (including the bKeepRunning flag) from your thread.  You will be setting the flag to FALSE in the CGui4UnexApp::ExitInstance function (along with other things; see below) to shut down the thread.

3. Add two synchronization objects: a waitable timer in your thread, and a regular old event stored as a HANDLE member variable in CGui4UnexApp.  Set the interval for the waitable timer to 10 seconds.  Use WaitForMultipleObjects at the bottom of the loop in your thread so that the wait is signalled on either the waitable timer (to restart the loop back to the beginning) or on the event which you'll also set in your CGui4UnexApp::ExitInstance function.

4. Change the way you start up the thread so the thread starts suspended.  Then set the thread's bAutoDelete member varialbe to FALSE and resume the thread.

5. Add the CGui4UnexApp::ExitInstance function, to set bKeepRunning to FALSE and signal the event.  Then enter a WaitForSingleObject on the thread's handle (this is the reason why you set the bAutoDelete variable to FALSE, so that thread object sticks around long enough to get a signal on its handle).  When the WaitForSingleObject signals, you'll know that the thread has shut down gracefully, so you can return from CGui4UnexApp::ExitInstance and let the remainder of the program shut down too.



So, your code would look something like this:

member variables to add to CGui4UnexApp:

CWinThread* m_pThread;
HANDLE m_hEvent;
BOOL bKeepRunning;


CGui4UnexApp::InitInstance()
{
 bKeepRunning = TRUE;
 m_hEvent = CreateEvent(NULL, FALSE, FALSE, NULL);
 m_pTHread = AfxBeginThread(ThreadFunc, (LPVOID) this, THREAD_PRIORITY_NORMAL, 0, CREATE_SUSPENDED);
 m_pThread->m_bAutoDelete=FALSE;
 m_pThread->ResumeThread();
}

CGui4UnexApp::ExitInstance()
{
 bKeepRunning = FALSE;
 SetEvent( m_hEvent);
 WaitForSingleObject( m_pThread->m_hThread, INFINITE);
 CloseHandle(m_hEvent);
}


For the ThreadFunc:

UINT CGui4UnexApp::ThreadFunc(LPVOID pParam)
{
     CGui4UnexApp* pApp = (CGui4UnexApp*) pParam;

     HANDLE hEvents[2];
     HANDLE hTimer = NULL;
    LARGE_INTEGER liDueTime;
    liDueTime.QuadPart=100000000;
    hTimer = CreateWaitableTimer(NULL, FALSE, NULL);
    SetWaitableTimer(hTimer, &liDueTime, 100000, NULL, NULL, 0);

     hEvents[0] = hTimer;
     hEvents[1] = pApp->m_hEvent;

 // your stuff
     CInternetSession session;
     CStdioFile* pFile = NULL;
     CString strLine;

     while (pApp->bKeepRunning)
    {
          /*
        //
          // do all your internet stuff here
          //
          */

          WaitForMultipleObject(2, hEvents, FALSE, INFINITE);

     }

     CloseHandle( hTimer);
     return 0;
}


I'm frankly not experienced on waitable timers, so it would be best if someone else chimes in here.  When I need to run a thread iteratively, I usually set an event in an OnTImer handler; but since this thread is owned by a CWinApp cobject and not a CWnd object, We can't do that here.

Mike
0
Highfive + Dolby Voice = No More Audio Complaints!

Poor audio quality is one of the top reasons people don’t use video conferencing. Get the crispest, clearest audio powered by Dolby Voice in every meeting. Highfive and Dolby Voice deliver the best video conferencing and audio experience for every meeting and every room.

 
LVL 19

Author Comment

by:mrwad99
ID: 11859436
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.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11885273
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*...
0
 
LVL 2

Expert Comment

by:MikeAThon
ID: 11885655
<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
0
 
LVL 19

Expert Comment

by:drichards
ID: 11885906
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?
0
 
LVL 19

Assisted Solution

by:drichards
drichards earned 250 total points
ID: 11886289
OK, what I see in VC6 is that as long as I provide synchronization to gracefully stop the thread, there is no leak.  If I just exit the app and allow the thread to terminate unnaturally, the leaks appear.  This does NOT happen in VC7.  All I get there is the CWinThread which is not cleaned up.

This is what I did to sync the thread shutdown:

1) Put an event handle in the dialog where the thread is started:

   HANDLE hEvent;

2) Then, in the thread startup,

    hEvent = ::CreateEvent(NULL, TRUE, FALSE, NULL);
    pThread = AfxBeginThread(ThreadFunc, hEvent);


3) Then in ThreadFunc:

UINT ThreadFunc(LPVOID pParam)
{
    HANDLE hEvent = (HANDLE) pParam;
     CInternetSession session;
 ...

4) Then in all paths to close dialog, add a call to:

    ::SetEvent(hEvent);
    ::WaitForSingleObject(pThread->m_hThread, INFINITE);
    ::CloseHandle(hEvent);


With this extra code, no leaks appear.  They appear to be artifacts of killing the thread with the big hammer and you probably don't need to worry about them.
0
 
LVL 2

Expert Comment

by:MikeAThon
ID: 11887313
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 http://www.experts-exchange.com/Programming/Programming_Languages/MFC/Q_21091944.html#11856282

But the OP said that he "tried the code you [meaning me] suggested and still had the leaks. "
0
 
LVL 19

Expert Comment

by:drichards
ID: 11887693
>> 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.
0
 
LVL 2

Expert Comment

by:MikeAThon
ID: 11888296
That's a clever way of avoiding my (more clumsy) waitable timer.

Regards,
Mike
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11921479
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.
0
 
LVL 19

Expert Comment

by:drichards
ID: 11922073
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.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11922421
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
0
 
LVL 2

Expert Comment

by:MikeAThon
ID: 11922796
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.

0
 
LVL 19

Expert Comment

by:drichards
ID: 11922988
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.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 11931452
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 :)
0
 

Expert Comment

by:pedja11
ID: 12151531
You should free memory with delete pFile after closing your file.
0
 
LVL 19

Author Comment

by:mrwad99
ID: 12151559
Yeah I do do cheers.
0

Featured Post

What Should I Do With This Threat Intelligence?

Are you wondering if you actually need threat intelligence? The answer is yes. We explain the basics for creating useful threat intelligence.

Join & Write a Comment

Introduction: The undo support, implementing a stack. Continuing from the eigth article about sudoku.   We need a mechanism to keep track of the digits entered so as to implement an undo mechanism.  This should be a ‘Last In First Out’ collec…
If you use Adobe Reader X it is possible you can't open OLE PDF documents in the standard. The reason is the 'save box mode' in adobe reader X. Many people think the protected Mode of adobe reader x is only to stop the write access. But this fe…
This video will show you how to get GIT to work in Eclipse.   It will walk you through how to install the EGit plugin in eclipse and how to checkout an existing repository.
When you create an app prototype with Adobe XD, you can insert system screens -- sharing or Control Center, for example -- with just a few clicks. This video shows you how. You can take the full course on Experts Exchange at http://bit.ly/XDcourse.

757 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

18 Experts available now in Live!

Get 1:1 Help Now