Critical Sections and Memory Fences / Barriers on a multi-processor system

Question

I have a Windows DLL (in C) that uses Critical Sections. A specific routine, which is called numerous times, needs to perform some initialization code the first time it is called, so I am using a Critical Section. However, since it is called so many times I am trying to avoid the overhead of entering the section each time it is called. It seems to be working, but I am wondering if there is a flaw considering memory barriers / fences when running on a multi-processor (Intel) system with an x64 OS? Here is the stripped down code:

int _isInitialized = FALSE;
CRITICAL_SECTION _InitLock = {0};

BOOL APIENTRY DllMain( HANDLE hModule, DWORD ul_reason_for_call, LPVOID lpReserved )
{
    ARM_SECTION_BEGIN(ul_reason_for_call)

    switch (ul_reason_for_call)
    {
    case (DLL_PROCESS_ATTACH):
        InitializeCriticalSection(&_InitLock);
        break;
    case (DLL_THREAD_ATTACH):
        break;
    case (DLL_THREAD_DETACH):
        break;
    case (DLL_PROCESS_DETACH):
        DeleteCriticalSection(&_InitLock);
        break;
    }
    return (TRUE);
}

int myproc(parameters...)
{
    if (!_isInitialized)        // check first time
    {
        EnterCriticalSection(&_InitLock);
        if (_isInitialized)        // check it again
        {    
            LeaveCriticalSection(&_InitLock);
            goto initialized;
        }
        ... do stuff ...
        _isInitialized = TRUE;
        LeaveCriticalSection(&_InitLock);
    }
initialized:
    ... do more stuff ...
    return(something)
}

user_2410359 · Answer

... if there is a flaw considering memory barriers / fences ... ?

Use volatile

Code certainly appears to risk using a stale value for _isInitialized in the 2nd test.

if (!_isInitialized) {
  EnterCriticalSection(&_InitLock);
  if (_isInitialized)                 // Risk

To insure a re-read of _isInitialized, use volatile. @JimmyB

// int _isInitialized = FALSE;
volatile int _isInitialized = FALSE;

Other Shared Data

Other data than _isInitialized assigned in the ... do stuff ... and used in the later ... do more stuff ... code risks the same problem due to an optimization may read other_data before the the first if (!_isInitialized).

Code could use volatile other_data. Unfortunately that may incur an unacceptable performance drag. Alternatives depend on what is inside stuff.

Style

I'd make _isInitialized local to the function, drop the _ and avoid the goto.

int myproc(parameters...) {
  static volatile int isInitialized = FALSE;

  if (!isInitialized) {
    EnterCriticalSection(&_InitLock);
    if (!isInitialized) {
      // ... do stuff ...
      isInitialized = TRUE;
    }
    LeaveCriticalSection(&_InitLock);
  }

  // ... do more stuff ...
  return(something)
}

Critical Sections and Memory Fences / Barriers on a multi-processor system

1 Answers