::EnterCriticalSection locking.
I have the following class:
Code:
class CAutoCriticalSection: public CRITICAL_SECTION
{
public:
CAutoCriticalSection ()
: m_nLockCount(0)
{
::InitializeCriticalSection(this);
}
~CAutoCriticalSection ()
{
::DeleteCriticalSection(this);
}
void Lock ()
{
::EnterCriticalSection(this);
m_nLockCount++;
}
void UnLock ()
{
::LeaveCriticalSection(this);
m_nLockCount--;
}
public :
int m_nLockCount;
};
Sometimes, although quite rarely, EnterCriticalSection(this) locks, i.e waits for an undetermined amount of time, and I was wondering if there is anyway to determine what the best course of action would be to release lock and output a diagnosis message to say that an unsuccessful attempt was made to enter the critical section after, say, 2 seconds.
Regards
John
Re: ::EnterCriticalSection locking.
Quote:
Sometimes, although quite rarely, EnterCriticalSection(this) locks, i.e waits for an undetermined amount of time,
Sounds like a dead-lock.
Quote:
and I was wondering if there is anyway to determine what the best course of action would be to release lock and output a diagnosis message to say that an unsuccessful attempt was made to enter the critical section after, say, 2 seconds.
You cannot do that with critical secions. You can TryEnterCriticalSection, but that will fail right away if the section is already locked.
One thing you might try out is replace the critical sections with events or mutextes. And use WaitForSingleObject with a timeout. Again that would be more resource consuming.
- petter
Re: ::EnterCriticalSection locking.
thanks for the reply Wildfrog... unfortunately, I'm unable to used any other synchronisation method as its part of the implemation already used in a rather big software project.
Let me ask you this question then: Is there a way just to time out after 2 secs upon unsuccessful entry to a critical section? This way, I can use the _RTL_CRITICAL_SECTION structure to output a diagnosis message that we can use to track the problem.
Regards
Re: ::EnterCriticalSection locking.
Re: ::EnterCriticalSection locking.
I agree with wildfrog that you probably have a deadlock.
That said, there are 2 ways to set timeouts for critical sections:
1) Systemwide. The registry value
HKLM\SYSTEM\CurrentControlSet\Control\SessionManager\CriticalSectionTimeout
The unit for this value is in seconds. You must set a value <= 3600 for this to take effect. The default value is 30 days, which isn't actually used.
2) Process specific. Set an option in your executable image file. You need to create a load configuration for your exe. Take a look at the struct IMAGE_LOAD_CONFIG_DIRECTORY32 which is defined in winnt.h. There's a field there CriticalSectionDefaultTimeout. This field is in milliseconds. Again, it must be smaller than an hour to take effect.
To get the linker to include a load configuration in your exe, you need to define a global variable like this:
Code:
extern "C" const IMAGE_LOAD_CONFIG_DIRECTORY32 _load_config_used = { /* initialize all the fields here */};
Last but not least, when you enable a timeout for critical sections, and a timeout occurs, the system will throw an exception of type STATUS_POSSIBLE_DEADLOCK (0xC0000194). So you need to catch that with __try/__except if you don't want your program to crash.
The documentation for EnterCriticalSection says about this "This function can raise EXCEPTION_POSSIBLE_DEADLOCK if the critical section is corrupt or deadlock detection is enabled. Do not handle this exception; either continue execution or debug the application."
Re: ::EnterCriticalSection locking.
Hi all,
Wildfrog, I've been reading that document for most of the day and, unfortunately due to the fact that I'm fairly inexperienced at multi-threading, I found the concept easy to understand, but it would be very difficult to implement.
Googler, your solution was great, but wouldn't be changing the registry to reflect the solution provided have a massive impact on other Critical Sections within the project? I guess this value within the registry goes for every single critical section you use in your existing projects, and, if you forget to change it back for when doing a new project, wouldn't that have serious consequences?
However, I do have one more question to ask: What is stopping me from using TryEnterCriticalSection as follows:
Code:
long lMaxTime = 2000;
long lCurrentTimeSlice = 0;
while (!TryEnterCriticalSection(m_blockObj))
{
if (lCurrentTimeSlice < lMaxTime)
{
Sleep (100);
lCurrentTimeSlice+=100;
}
else
{
// Report that there is a problem here by using CRITICAL_SECTION's DebugInfo
break; // Good idea??
}
}
What, if any, is the side-effects of determining if there is a problem with the critical section?
Regards
Re: ::EnterCriticalSection locking.
Code:
::InitializeCriticalSection(this);
This is dangerous code. InitializeCriticalSection expects an address to a CRITICAL_SECTION structure, not to some class object. The final layout of the class object may be highly dependent on how the compiler arranges the data, the vtables etc..
Why don't you do this instead:
Code:
class CAutoCriticalSection
{
private:
CRITICAL_SECTION m_cs;
public:
CAutoCriticalSection ()
: m_nLockCount(0)
{
::InitializeCriticalSection(&m_cs);
}
~CAutoCriticalSection ()
{
::DeleteCriticalSection(&m_cs);
}
void Lock ()
{
::EnterCriticalSection(&m_cs);
m_nLockCount++;
}
void UnLock ()
{
::LeaveCriticalSection(&m_cs);
m_nLockCount--;
}
public :
int m_nLockCount;
};
Re: ::EnterCriticalSection locking.
Hi Kirants,
Thanks for the response. Thats the way I would do it too. But as this is someone elses code, and has been in place for quite some time now (3 to 4 years.) with out any problems, do you think that what you have described maybe the reason as to why it has suddenly stopped working, i.e, the EnterCriticalSection has deadlocked?
Apologies for the seemingly silly questions, but as I'm fairly inexperienced in multithreading issues, I'm looking for as much as possible from people's experiences before I'm confident enough to commit to code and tell my manager that the original soultion is incorrect.
Regards
Re: ::EnterCriticalSection locking.
Not sure.. Deadlocks can be very difficult to diagnose at times and most often, one has to sit with a paper and pencil and figure it out, not ignoring the silliest of the reasons.
Let me ask you this, does your system( your app) have threads that are fighting for each other resources ? i.e. A fighting for B's and B's in turn fighting for A's ? Are there multiple critical section objects, or is there just one, being fought for by all the threads ?
Also, how often does it deadlock ? DO you have to run it for hours together to get to such a situation ?
Quote:
but as I'm fairly inexperienced in multithreading issues
Well, this is your chance to get your feet wet.. you'll come out wiser for sure after you have tackled this issue ;)
Re: ::EnterCriticalSection locking.
Quote:
Originally Posted by kirants
Let me ask you this, does your system( your app) have threads that are fighting for each other resources ? i.e. A fighting for B's and B's in turn fighting for A's ? Are there multiple critical section objects, or is there just one, being fought for by all the threads ?
From what I've seen, there are quite a few Critical Sections - too many to mention. But this particular critical section, there are quite a few number of threads (or should I say objects?) competing for this particular Critical section.
Quote:
Originally Posted by kirants
Also, how often does it deadlock ? DO you have to run it for hours together to get to such a situation ?
Since I'm new in this job, I did ask this question and they said that this is the first time its happened, and they've been running it for about 2-3 years!! :eek:
Quote:
Originally Posted by kirants
Well, this is your chance to get your feet wet.. you'll come out wiser for sure after you have tackled this issue ;)
Well, I'm up for the challenge, hence the reason why I recently took up the new position - Its one of the few areas of my current skill set that really sucks! LOL!
Oh, btw, Know of any great books on multi threading?
Regards
Re: ::EnterCriticalSection locking.
Quote:
Originally Posted by kirants
Code:
::InitializeCriticalSection(this);
This is dangerous code. InitializeCriticalSection expects an address to a CRITICAL_SECTION structure, not to some class object. The final layout of the class object may be highly dependent on how the compiler arranges the data, the vtables etc..
There's nothing dangerous about it. InitializeCriticalSection() has a formal parameter of type CRITICAL_SECTION*. The actual parameter this is a CAutoCriticalSection*. The compiler performs a derived* to base* coversion as usual and will generate the correct address by adding the appropriate offset if necessary.
To Vaderman:
1) The code you posted in reply #6 is not immune to starvation. The thread executing this loop may never be able to lock the critical section, but not due to a deadlock, but rather because it keeps losing out to other threads. EnterCriticalSection() is immune to starvation - any thread calling it will eventually lock the crit sect (unless there's a deadlock of course...).
2) I understand your apprehension about modifying the crit sect timeout for the entire system via the session manager. Like I said, there's a way to do it specifically for your app. I've tried testing it, but there seems to be some issues with getting the linker to embed a load configuration. If I'm successful, I'll post code on how to do this right.
3) I'd like to take back what I said about catching STATUS_POSSIBLE_DEADLOCK exceptions. If you enable deadlock detection via timeouts, you shouldn't handle these exceptions. Instead, you should let them crash your app. Then you'll get a chance to run your just-in-time debugger, and once in the debugger you can analyze the deadlock and figure out why it's happening.
4) The only generally correct way to resolve a deadlock after it happens is to kill all the threads participating in the deadlock (provided you can identify them...). Deadlocks should be fixed by figuring out how your program logic causes them to happen and then changing the program logic to prevent deadlocks from happening in the first place. That's what the timeout mechanism I mentioned is for - it's a debugging aid, not a means for "handling" deadlocks.
Re: ::EnterCriticalSection locking.
Ok, here is how to set critical section timeouts for your app:
Add a C source file to your project called loadcfg.c with contents as follows
Code:
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#ifdef __cplusplus
#error This source file must be compiled as "C Code" (/TC)
#endif
const IMAGE_LOAD_CONFIG_DIRECTORY32 _load_config_used = {
sizeof(IMAGE_LOAD_CONFIG_DIRECTORY32),
0,
0,
0,
0,
0,
2000, // CriticalSectionDefaultTimeout msec
0,
0,
0,
0,
0,
0,
0,
0,
0,
0,
0,
0,
0
};
I set a timeout of 2000 milliseconds, but you can change it.
The name "loadcfg" isn't important, but it must be a C source file. For some reason, if it's a cpp source file the linker doesn't embed the load configuration (even if I use extern "C"). So the file should end in ".c" and in the project settings the "Compile As" setting should be set to either "Default", or "Compile as C Code (/TC)" at least for this specific source file.
I put an #error directive in there just to guard against compiling it as a cpp file by accident.
I tested this and caused a deliberate timeout on a critical section. It popped up the just-in-time debugger which told me which thread was holding the critical section and which thread was hung on it.
Re: ::EnterCriticalSection locking.
Quote:
Originally Posted by googler
There's nothing dangerous about it. InitializeCriticalSection() has a formal parameter of type CRITICAL_SECTION*. The actual parameter this is a CAutoCriticalSection*. The compiler performs a derived* to base* coversion as usual and will generate the correct address by adding the appropriate offset if necessary.
That's right. Thanks for correcting :wave:
Quote:
The only generally correct way to resolve a deadlock after it happens is to kill all the threads participating in the deadlock (provided you can identify them...). Deadlocks should be fixed by figuring out how your program logic causes them to happen and then changing the program logic to prevent deadlocks from happening in the first place. That's what the timeout mechanism I mentioned is for - it's a debugging aid, not a means for "handling" deadlocks.
Very true.. It's important that you troubleshoot the cause. Who knows it might as well reveal something even worse. A thorough review of the threads and their interaction is a valuable investment
Re: ::EnterCriticalSection locking.
Quote:
Originally Posted by googler
The name "loadcfg" isn't important, but it must be a C source file. For some reason, if it's a cpp source file the linker doesn't embed the load configuration (even if I use extern "C").
I've got no trouble compiling/linking it using extern "C" (in a cpp-file)
Code:
extern "C" const IMAGE_LOAD_CONFIG_DIRECTORY _load_config_used =
{
sizeof(IMAGE_LOAD_CONFIG_DIRECTORY),
0,
0,
0,
0,
0,
2000, // CriticalSectionDefaultTimeout msec
0,
0,
0,
0,
0,
0,
0,
0,
0,
0
};
EDIT: Using VC++8.0 that is.
- petter
Re: ::EnterCriticalSection locking.
Yes, it compiles. But try doing "dumpbin /headers" on your exe file. In the "OPTIONAL HEADER VALUES" area, right below "number of directories" see if you have an RVA for "Load Configuration Directory". There should be an RVA there to the copy of the struct body (which should be in the .rdata section). If this pointer is not set, the system won't recognize it. For some reason, when compiling with extern "C" in a cpp file, this pointer is null. When I compile as a C file, this pointer gets set. I looked in the CRT sources, and they have a source file called loadcfg.c which also creates a load config this way and is a C file. This is really weird...
Re: ::EnterCriticalSection locking.
Quote:
Originally Posted by googler
Yes, it compiles. But try doing "dumpbin /headers" on your exe file. In the "OPTIONAL HEADER VALUES" area, right below "number of directories" see if you have an RVA for "Load Configuration Directory". There should be an RVA there to the copy of the struct body (which should be in the .rdata section).
You're right about that, I tested using MapAndLoad(...)/GetImageConfigInformtion(...) and it also claims that the file does not contain any load configuration.
Quote:
Originally Posted by googler
If this pointer is not set, the system won't recognize it.
Well, thats the strange thing. My critical sections as still timing out after the specified milliseconds.
This needs further investigation...
- petter
Re: ::EnterCriticalSection locking.
I tried it again. Yesterday, I wrote it like this:
Code:
#ifdef __cplusplus
extern "C" {
#endif
const IMAGE_LOAD_CONFIG_DIRECTORY32 _load_config_used = {
/* blah blah */
};
#ifdef __cplusplus
}
#endif
And this wasn't getting a load configuration set when compiled as CPP. The critical sections weren't timing out either.
Today, I tried it the other way...
Code:
extern "C" const IMAGE_LOAD_CONFIG_DIRECTORY32 _load_config_used = {
/* blah blah */
};
and guess what? dumpbin shows there is a load configuration, and the critical sections are timing out.
So I checked the help for extern "C" and I noticed that it talks about declarations. So it seems that if you just have a definition inside an extern "C" {} block, they don't get extern "C"ed !!!! But if you write a definition with its own private extern "C", it does get extern "C"ed. Talk about strange...
So, I guess the following version should work whether compiling as C or C++:
Code:
#ifdef __cplusplus
extern "C"
#endif
const IMAGE_LOAD_CONFIG_DIRECTORY32 _load_config_used = {
/* blah blah */
};
Re: ::EnterCriticalSection locking.
Vaderman, before modifying the default cs timeout, I would suggest adding some trace debug statements so that you can pinpoint which objects are getting locked and/or if making sure you don't have any circular dependencies.
You mention previous owners of the code say that this code worked fine before. My first reaction to that is 'R i ght.' However, if it did work fine before, can you determine what changed in the code that might cause it to fail? If nothing has changed in the code, then don't assume it ever worked correctly (it might just be that this flaw wasn't uncovered before or something else changed in the system like different hardware or increased load).
As far as locking up... Are you sure you are always calling .Unlock(). Are there any code paths that prevent Unlock from always getting called? Note: if you kill a thread that shares the critical section, this will prevent the thread from calling .Unlock. Could this be occuring?
Someone else mentioned thread starvation. How many threads are accessing the cs? If there are many threads, you may consider using a thread pool mechanism to restrict the number of threads (use the QueueUserWorkItem api or the queuing approach mentioned earlier).
I would suggest going over your code carefully. Here are a few things to look for:
- Make sure that you aren't killing any threads that share a lock object..
- Make sure that .Unlock ALWAYS gets called.
- Next, I would look at how long you are holding onto the lock. Are you locking, accessing the resource, and then immediately unlocking? Or are you locking, accessing the resource, performing some other lengthy work, then unlocking the resource?
- If so, ask yourself if this work needs to be done while the resource is locked or if you can perform the work after the resource has been unlocked. Sometimes, the lock is held too long because of a coding oversite. Other times, you need to hold on to the lock because the resource is being updated during the work operation. In the latter case, you can often reduce contention by making a copy of the resource that the long operation uses. You would lock, copy, unlock and perform the lengthy operation; then you would lock, update the original from the copy, and unlock. At first it might seem like doubling the amount of locking per work operation isn't ideal, but two short locks are better than one long lock.
- If most of the time you are reading (with very few writes), consider moving to a singlewriter/multiple reader locking object rather than a critical section (where every lock is treated as a write lock).
Arjay
