Re: Multithread heap assertion failure(Continued)
- From: Joseph M. Newcomer <newcomer@xxxxxxxxxxxx>
- Date: Sat, 11 Aug 2007 11:51:21 -0400
See below...
On Thu, 09 Aug 2007 19:20:15 -0700, Zhiguo <ZhiguoYoung@xxxxxxxxx> wrote:
Oh sorry, I made a mistake in the test code.****
The source file is as follows:
#include <afx.h>
#include <afxmt.h>
#include <afxwin.h> // MFC core and standard components
/*
#include <afxext.h> // MFC extensions
#include <afxdtctl.h> // MFC support for Internet Explorer 4 Common
Controls
#ifndef _AFX_NO_AFXCMN_SUPPORT
#include <afxcmn.h> // MFC support for Windows Common Controls
#endif // _AFX_NO_AFXCMN_SUPPORT
#include <windows.h>
#include "iocrPage.h"*/
#include <iostream>
using namespace std;
class CTestThread: public CWinThread{
public:
long timeout;
CTestThread(){}
virtual ~CTestThread(){}
int Run(){
cout << "run..." << endl;
cout is not thread-safe. Therefore, any attempt to use it must be protected by a
synchronization primitive (such as a CRITICAL_SECTION). Otherwise, you can experience
heap damage because two or more threads will actually overwrite buffers and damage the
heap (I've seen this happen to people who used cout in this fashion).
Generally, there is no need to override the Run() method of a thread. You would be better
off using worker threads in this case.
****
Sleep(timeout);****
return 1;
When Run() returns, ExitInstance is called
****
}****
BOOL InitInstance(){
cout << "init..." << endl;
/*
CoInitializeEx(NULL, COINIT_MULTITHREADED);
char szDllName[_MAX_PATH] = "fzrslib.dll";
TCHAR szDllName[MAX_PATH] = _T("fzrslib.dll");
program Unicode-aware!
*****
HMODULE m_hInst = LoadLibrary(szDllName);****
DWORD dwErr = GetLastError();
WRONG! The value of dwErr is normally changed ONLY if there is an error; therefore, you
cannot assume that it will have meaning if the LoadLibrary succeeded! The ONLY correct
way to do this is to test to see if the LoadLibrary failed, and ONLY THEN can you know if
GetLastError is going to give you anything meaningful!
if(m_hInst != NULL)
{ /* LoadLibrary failed */
DWORD dwErr = ::GetLastError();
if (dwErr!=0) {***
cout << "init error..." << endl;
return FALSE;
}
} /* LoadLibrary failed */
LPCreateRecogPageInstanceFun m_pCreator =****
(LPCreateRecogPageInstanceFun)GetProcAddress(m_hInst,
"CreateRecogPageInstance");
Why not name the variable after the function name, instead of using a silly name like
m_pCreator? I would have written
CreateRecogPageInstance =
(LPCreateRecogPageInstanceFun)GetProcAddress(...)
Also, it is EXCEPTIONALLY POOR STYLE BEYOND ALL HOPE to EVER use a name like m_ for a
local variable!!!! If you don't understand how to use these naming conventions, DON'T USE
THEM AT ALL!!!! m_ is reserved STRICTLY for member variables of a class!
***
GetProcAddress(m_hInst, "CreateRecogPageInstance");****
IRecognizePage * m_pPage;
HRESULT m_hr = (*m_pCreator)(&m_pPage);
This is obsolete syntax, and has been obsolete for decades. Do not use it. For example,
I would have called my variable CreateRecogPageInstance, and I would have written
HRESULT hr = CreateRecogPageInstance(&m_pPage);
and that is vastly more readable, since (a) it has the actual function name (b) it doesn't
clutter it up with useless (*) notation. Note that m_hr is again a horrible misuse of the
naming conventions, because it is a local variable, not a member variable of the class!
****
cout << "init ok..." << endl;*/****
And you can't use cout without using synchronization with the thread. The alternative is
to never use cout in any thread but the main GUI thread.
****
return TRUE;****
}
int ExitInstance(){
cout << "exit inst..." << endl;
Uses of cout must be synchronized
****
/*****
if (m_pPage) {
m_pPage->ClearCurrentResult();
m_pPage->Release();
}
if (m_hInst) {
FreeLibrary((HINSTANCE)m_hInst);
}
return TRUE;
CoUninitialize();*/
return 0;
}
};
int main(){
int nWorkerNum = 2;
CTestThread * m_pThread = new CTestThread[nWorkerNum];
HANDLE * m_pThreadHandle = new HANDLE[nWorkerNum];
for(int i = 0; i < nWorkerNum; i ++){
m_pThread[i].CreateThread(CREATE_SUSPENDED);
m_pThread[i].m_bAutoDelete = FALSE;
m_pThreadHandle[i] = m_pThread[i].m_hThread;
}
m_pThread[0].timeout = 1;
This does not set a 1ms timeout; it sets a 15ms timeout. And that is far too often
anyway.
****
m_pThread[1].timeout = 3000;****
for(int i = 0; i < nWorkerNum; i ++){
m_pThread[i].ResumeThread();
}
WaitForMultipleObjects(nWorkerNum, m_pThreadHandle, FALSE, INFINITE);
The waiting-for-thread-completion is good.
****
cout << "here" << endl; cout.flush();****
clock_t s = clock();
WaitForMultipleObjects(nWorkerNum, m_pThreadHandle, TRUE, 2500);
clock_t e = clock();
Note that clock_t has lousy resolution for timing anything of any value, since it has no
better resolution (independent of what you see as CLOCKS_PER_SEC) than 15 ms. So unless
your timing is out into seconds, it won't give you meaningful results. It is probably
untrustworthy for any value < 1sec.
****
cout << "####" << (e - s) << endl;****
cout << "there" << endl; cout.flush();
for(int i = 0; i < nWorkerNum; i ++){
m_pThread[i].ExitInstance();
}
This, however, is completely wrong. The ExitInstance code for a thread must be executed
by the thread, and not called by some other thread. Get rid of this, because it will
simply do the wrong thing. ExitInstance has already been called once, when your Run()
method returned, and calling it a second time, from a completely different thread, will
likely result in the kind of error you are seeing, unless it is just cout corrupting the
heap.
joe
****
Joseph M. Newcomer [MVP]
delete [] m_pThreadHandle;
delete [] m_pThread;
cout << "Press any key to conitnue..." << endl;
cout.flush();
char c; cin >> c;
}
email: newcomer@xxxxxxxxxxxx
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
.
- Follow-Ups:
- Re: Multithread heap assertion failure(Continued)
- From: Norbert Unterberg
- Re: Multithread heap assertion failure(Continued)
- References:
- Multithread heap assertion failure(Continued)
- From: Zhiguo
- Re: Multithread heap assertion failure(Continued)
- From: Zhiguo
- Multithread heap assertion failure(Continued)
- Prev by Date: Re: undivideable action?
- Next by Date: Re: Why my component does not receive windows messages.
- Previous by thread: Re: Multithread heap assertion failure(Continued)
- Next by thread: Re: Multithread heap assertion failure(Continued)
- Index(es):
Relevant Pages
|