::EnterCriticalSection locking.

[Vaderman]

I have the following class:

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

[wildfrog]

Sometimes, although quite rarely, EnterCriticalSection(this) locks, i.e waits for an undetermined amount of time,

Sounds like a dead-lock.

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

[Vaderman]

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

[wildfrog]

Maybe this Break Free of Code Deadlocks in Critical Sections Under Windows can give you some directions.

- petter

[googler]

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:

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."

[Vaderman]

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:

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

[kirants]

::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:

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;
};

[Vaderman]

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

[kirants]

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 ?

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

[Vaderman]

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.

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!!

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

[googler]

::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.

[googler]

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

#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.

[kirants]

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

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

[wildfrog]

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)

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

[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). 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...