More help sought for my Monitoring and Controlling Worker Thread article...

Ben Aldhouse

I'm currently working on a rewrite of my most recent article Monitoring and Controlling a Recursing Function in a Worker Thread and would love to know your opinions on the fowing block of code I use in the worker thread...

            do {
                Sleep(1);
            }while (pMyView->m\_strCurFile != pMyView->m\_strConfirm);

This has the effect of slowing things up enough so that the main thread can display a gratifyingly fast moving display of files flash past in the text box on the dialog. I was going to say that, without it, the dialag doesn;t display any file information until the thread has stopped running but I've just tried it and, in fact, with out it I get an Access Violation error! It doesn't seem like a very nice bit of code to me. Could it be replaced with WaitForSingleObject or something, somehow? Interestingly, if I try and speed the thread up by putting in an extra mod test so that it only runs every tenth loop...

if (n % 10 ==0){
            do {
                Sleep(1);
            }while (pMyView->m\_strCurFile != pMyView->m\_strConfirm);
}

...this also causes problems...

Stephen Hewitt

In general Sleep is bad form, although as with any rule of thumb there are exceptions. Unfortunately, from what I've seen your code isn't one of them.

Ben Aldhouse wrote:

It doesn't seem like a very nice bit of code to me. Could it be replaced with WaitForSingleObject or something, somehow?

But what's you're problem? From what I gather, what you should be worried about is what's causing the access violation. Never stop a crash by poking around blindly till it stops: lose the Sleep and find the cause of the real problem (the access violation). The first thing I'd do is kill the Sleep, run the code under a debugger and get a stack trace of the access violation. I suggest posing it.

Steve

Lost User

I took a brief look at your article. I have some humble comments/observations. 1.) Yes you should get rid of the call to Sleep. I have never found any use for this API call and in my opinion it is probably the most abused function on planet Earth. I recommend the following: a) Add: HANDLE hEvent; to your CFormView. b) Call hEvent = CreateEvent(NULL,FALSE,FALSE,"Your-GUID"); in your constructor. c) Call SetEvent(hEvent); in your OnCURFILEEVENT message handler. d) Replace the Sleep(1) with: WaitForSingleObject(hEvent,1000); and then reset the event with ResetEvent(hEvent); immediately afterwards. 2.) Regarding your access violation... I instantly recognized the problem. You are incorrectly accessing a CString object that lives inside your CFormView from the worker thread. CString is NOT thread-safe. You need to replace the pMyView->m_strCurFile assignment with a message function such as SendMessageTimeout[^] and this will fix the Access Violation in your article. Best Wishes, -David Delaune

Ben Aldhouse

Thanks for this Stephen, you have hit the nail on the head, I think, regarding the access violation. I have had a reply indicating that my use of CString objects is causing this. I will investigate this.

Ben Aldhouse

I am very grateful to you, David, for looking at my code. You only looked at it briefly but were able to provide me with these very specific points of help. It's humbling for me because it will take longer for me to try them out than it took for you to look through my code, understand what I was trying to do and make these suggestions. I will go through them carefully, though. You clearly know what you are writing about.

Ben Aldhouse

Hi Dave, I've got the project working with the following alterations... Here is the sending code in MakeListing...

CString\* pString = new CString(FindFileData.cFileName);
SendMessageTimeout(pMyView->GetSafeHwnd(), CURFILEEVENT, (WPARAM)pString, 0, 0,  1000, 0);
delete pString;

Here is the receiving function...

LRESULT CRecThread2008_64Dlg::OnCURFILEEVENT(UINT wParam, LONG lParam)
{

CString\* pString = (CString\*)wParam;
CString tempStr = pString->GetBuffer();

m\_iNoFiles++;
CString tString=LPCTSTR("");
tString.Format(\_T("%d"),m\_iNoFiles);
m\_strNoFiles = tString;

m\_strFileList= tempStr + \_T("\\r\\n")+ m\_strFileList;
m\_strFileList = m\_strFileList.Left(500);

UpdateData(FALSE);    
return 0;

}

And it works without the sleep block - so I have got rid of that! I think I getting near to a point where I can start to rewrite the article... Thank you for your help with this, Ben.

Ben Aldhouse

David, the code and article is rewritten, now. Thanks so much for your help. I used SendMessageTimout and it worked nicely! Thanks, Ben.