Invalid access to memory location using GlobalFree

[superbonzo]

What's this critical section for?

but the l_State variable is assigned in the window message pump thread, so unless you can prove that such assignments and comparisons are atomic synchronization is needed here.

[Paul McKenzie]

Thanks for your comments and suggestions. Yes, I think your second point is the problem as well - although the program has got a load of other problems besides, now that I've had time to look more closely through the code to try to understand the flow.

Maybe it would be easier to figure out what the ultimate goal of that piece of code is supposed to be instead of trying to understand the flow. The problem with trying to understand the flow of code that is, for all practical purposes with code like this, it leaves you open to miss something subtle (the subtlety occurs only because of the spaghetti-like logic that was used).

I recently had to change code that was so badly written, I had to redo the entire module. I wasn't going to waste time trying to figure out all the twists and turns, when I already knew what the code was supposed to accomplish.

Regards,

Paul McKenzie

[OReubens]

but the l_State variable is assigned in the window message pump thread, so unless you can prove that such assignments and comparisons are atomic synchronization is needed here.

Correct.
Although there are better ways to achieve the same effect.

If you assume the VC compiler (and some others) then marking the bool as volatile and making sure it's properly aligned would be sufficient.
if you can't assume a particular brand of compiler and a particular way it handles volatile (the spec guarantees nothing) you need explicit synchronisation. Even in that case, there's better ways around it.

But the whole idea here of creating a thread and then communicating with the main thread via timer messages makes it obvious that the guy that wrote this was just clueless on what he was actually trying to do.

[2kaud]

Yes well, I posted the above code snippets for you all to have a laugh - at the moment I could do with one myself. The guys that wrote it have all long gone and until I started to look in detail at the program no one had a clue it was so badly coded (which says something about management, project control etc etc - further heads are rolling over this). We've now looking at other programs these guys had messed with to get an idea of how big a problem we really have.

I won't try to disect the code snippets much. I'm writing a completely new program to replace this mess from the ground up and this sort of mad programming won't feature. As far as I can tell, this code had two purposes. One was to check that memory wasn't being corrupted - which comes back to my original posting question re error with GlobalFree which started off this whole episode - and second was to make sure that the message loop in the main program wasn't waiting around for processing to complete whilst dealing with a message and hence not making the program responsive to user input/mouse etc. If a message took too long to process then the hearbeat would fail and an error message displayed. Practically every message in the main WndProc function that is processed within the program starts a new thread!!!! As I said before, I've never in all my years programming seen anything like it - and I hope I never will.