Re: Enter / Leave Critical Section in different threads
On Thu, 21 Jun 2007 13:31:19 -0700, juping.jin@gmail.com wrote:
First, I thought that it was wrong to enter a critical section from
one thread and then leave the same critical section from another
thread.
It is. The documentation for LeaveCriticalSection says:
http://msdn2.microsoft.com/en-us/library/ms684169.aspx
<q>
If a thread calls LeaveCriticalSection when it does not have ownership of
the specified critical section object, an error occurs that may cause
another thread using EnterCriticalSection to wait indefinitely.
</q>
I wrote a small program and it indicates that there is no
error to do so:
#include <windows.h>
#include <process.h>
#include <stdio.h>
struct threadparameter {
CRITICAL_SECTION *criticalsection;
int threadid;
};
// enter / leave critical section in different threads
void testcriticalsection(void *params){
threadparameter *parameter = (threadparameter*)params;
int threadid = parameter->threadid;
switch (threadid) {
case 0:
EnterCriticalSection(parameter->criticalsection);
printf("thread %d entered critical section, error code: %d\n",
threadid,GetLastError());
Sleep(9000);
break;
case 1:
LeaveCriticalSection(parameter->criticalsection);
printf("thread %d left critical section, error code: %d\n",
threadid,GetLastError());
Sleep(5000);
break;
case 2:
printf("thread %d enter critical section\n", threadid);
EnterCriticalSection(parameter->criticalsection);
printf("thread %d entered critical section, error code: %d\n",
threadid,GetLastError());
break;
}
printf("thread %d is done\n", threadid);
}
int main() {
CRITICAL_SECTION criticalsection;
InitializeCriticalSection(&criticalsection);
printf("testing different thread enter / leave critical section\n");
const int threadcount = 3;
threadparameter param[threadcount];
HANDLE th[threadcount];
for (int i = 0; i<threadcount; ++i) {
param[i].threadid = i;
param[i].criticalsection=&criticalsection;
th[i] = (HANDLE)_beginthread(testcriticalsection,0, ¶m[i]);
}
Sleep(1000);
WaitForMultipleObjects(2, th, TRUE, INFINITE);
DeleteCriticalSection(&criticalsection);
printf("main exits\n");
return 0;
}
The first thread call EnterCriticalSection, the second calls Leave and
third calls Enter again. If the Leave call in the second thread has no
effect (since it doesn't own it), the third thread should be blocked.
But it was NOT. If the second thread doesn't call Leave, the third
thread IS blocked which is as expected. To prevent critical section
become undefined (when its owner thread dies), I put some sleep call
in the code (certainly, real application should not use or minimize
Sleep).
Why a thread doesn't own a critical section could release the lock? Is
there something wrong in the code so this is not a valid test?
Several things are wrong:
1. You've defined a CRITICAL_SECTION on the stack, which means it goes away
when the enclosing scope is exited. It may not be a problem here, but there
was no reason not to make it a global.
2. You are relying on the threads executing in a certain order, but there's
no guarantee thread 0 executes before thread 1 and 1 before 2. So you have
multiple race conditions you need to eliminate.
3. You want sequences such as LeaveCriticalSection/printf to be atomic, but
they're not. Thus, your output can be interleaved in confusing ways.
4. You are using _beginthread and saving the returned HANDLE for later use
in WFMO. This is a bad idea because threads started with _beginthread close
their handles when they terminate, which is the same mistake MFC made with
CWinThread, which I talk about here:
http://members.cox.net/doug_web/threads.htm#Q1
Read the documentation for some other reasons why you should prefer
_beginthreadex:
http://msdn2.microsoft.com/en-us/library/kdzttdcb(VS.80).aspx
5. You think GetLastError is meaningful for the CRITICAL_SECTIONfunctions,
but the documentation is silent on the issue, which means it isn't.
Another issue illustrated in the code is WaitForMultipleObject. The
first parameter is the number of objects in handle array. in the above
example, the object count should be 3. However, if I put it 3 over
there, the call always returns as soon as ONE thread is done.
So, my second question is why WaitForMultipleObject doesn't wait for
all objects to signal?
Given that the handles may be closed by the terminating threads during the
WFMO call, the code is undefined. Using _beginthreadex should fix this.
It is quite possible that the small program has a fundamental fault
and hence, above issues are just my fault. In that case, I am eagerly
waiting for someone to point it out.
The most fundamental flaw is that it's testing something that's documented
not to work. Addressing the problems described above may be a useful
exercise, but suppose you "prove" that CRITICAL_SECTIONs don't have thread
affinity. You're still left with the documentation that says they do.
--
Doug Harrison
Visual C++ MVP