Critical Sections and Read operations.

[shoppinit]

If I had several threads that could modify a single variable, then I would enclose all the write operations in critical sections. If only one thread can modify the variable but the others can read it, do the read operations need to be enclosed in a critical section?

This question has been bugging me for a while and up to now I'm enclosing all read and write operations of a variable in a critical section. Maybe this is overkill?

[ProgrammerC++]

If it's just one variable that is changing then Critical Sections seem overkill indeed

[D_Drmmr]

If I had several threads that could modify a single variable, then I would enclose all the write operations in critical sections. If only one thread can modify the variable but the others can read it, do the read operations need to be enclosed in a critical section?

If multiple threads access a variable where at least one of the threads writes to it, then all threads must synchronize access to the variable. Otherwise, you have a race condition.

[shoppinit]

OK, I see.

The case in point is a bool bThreadContinue that I test to see if a flag has been set to let the thread terminate. I'm not so concerned about race conditions in this case, but rather access violations...

[ProgrammerC++]

A race condition can't occur in this scenario.

An access violation can not occur if it's a global static boolean. However, it's different in other cases, I suggest something like:

pseudo code

Destructor::~Destructor()
{
   bThreadContinue = FALSE; //Local var of class

   for all threads
   {
        ResumeThread( HandleToThread ); // if the thread is in suspended mode
        WaitForSingleObject( HandleToThread, INFINITE );
        CloseHandle( HandleToThread );
   }
}

This way, no access violation to the boolean can occur while all the threads are closing

[D_Drmmr]

The case in point is a bool bThreadContinue that I test to see if a flag has been set to let the thread terminate. I'm not so concerned about race conditions in this case, but rather access violations...

AFAIK, to cause an access violation you will need to do some erroneous pointer arithmetics (either directly or indirectly). If only a bool is involved, I don't see that happening.

You should be worried about race conditions, however. Even with 'just' a bool, you can mess up.

[shoppinit]

I was just wondering about threads running on different cores accessing the same memory area. On a single core processor, I agree that there wouldn't be a problem. I think I'll stick to using critical sections, just to be on the safe side

[D_Drmmr]

If it's just one variable that is changing then Critical Sections seem overkill indeed

A race condition can't occur in this scenario.

I don't know if it's because you don't have much experience with multi-threading, but I don't consider this to be very good advice.

It's not that difficult to create a race condition using just a bool variable. Given that you haven't seen a single line of code, how do you conclude that a race condition cannot occur?

Consider

bool myBool = false; // shared resource

// multiple threads running this function
void ThreadFunction()
{
    if (!myBool) {
        myBool = true;
        // do something that should only be done once
    }
}

A naive programmer might think that the code inside the if block won't get executed by more that one thread, but any number of threads running this function can evaluate the if clause before any of them set the bool to true in the first statement in the if clause. If you don't call that a race condition, IMO you are using a useless definition of the term.

[ProgrammerC++]

The thread start explicitly said he only changes the boolean in 1 thread.

[D_Drmmr]

The thread start explicitly said he only changes the boolean in 1 thread.

Then you still didn't get it.

If multiple threads access a variable where at least one of the threads writes to it, then all threads must synchronize access to the variable. Otherwise, you have a race condition.

[ProgrammerC++]

You can only get a race condition when there's two or more threads writing it

[Arjay]

You can only get a race condition when there's two or more threads writing it

Not so. You get a race condition if one or more thread write, while other threads are reading (or writing).

Why is thread synchronization so difficult?

[Codeplug]

>> You can only get a race condition when there's two or more threads writing it
No. You only need two threads - one reading and one writing. Without proper synchronization you have no guarantee that a write in thread A will even be seen in thread B.

[Arjay steals my thunder...]

gg

[ProgrammerC++]

I think both of you above me don't know that a race condition can't occur when there's 1 thread writing to 1 boolean and all other threads only read the boolean

[ProgrammerC++]

bad (well, compared to 'good' below, this still works but its slower since you have to go into the useless critical section all the time):

//psuedo code

//globals
CRITICAL_SECTION critsec;
bool closeworkerthreads = false;

bool getit()
{
   EnterCriticalSection( &critsec );

   bool to_return = closeworkterthreads;

   LeaveCriticalSection( &critsec );

   return to_return;
}

void setit( bool b )
{
   EnterCriticalSection( &critsec );

   closeworkterthreads = b;

   LeaveCriticalSection( &critsec );
}

DWORD WINAPI workerthread()
{
     while( getit() == false )
     {
         //... do some work

         Sleep( 100 );
     }
}

int main()
{
   InitializeCriticalSection( &critsec );

    HANDLE handles[8];
    handles[0] = CreateThread( workerthread );
    handles[1] = CreateThread( workerthread );
    handles[2] = CreateThread( workerthread );
    handles[3] = CreateThread( workerthread );
    handles[4] = CreateThread( workerthread );
    handles[5] = CreateThread( workerthread );
    handles[6] = CreateThread( workerthread );
    handles[7] = CreateThread( workerthread );

    Sleep( 5000 ); //wait 5 seconds before writing to it
    setit( true ); //manager thread decides all worker threads can close

    WaitForMultipleObjects( handles ); //Wait till all threads are closed

    DeleteCriticalSection( &critsec );
}

good:

//psuedo code

//globals
bool closeworkerthreads = false;

DWORD WINAPI workerthread()
{
     while( closeworkerthreads == false )
     {
         //... do some work

         Sleep( 100 );
     }
}

int main()
{
    HANDLE handles[8];
    handles[0] = CreateThread( workerthread );
    handles[1] = CreateThread( workerthread );
    handles[2] = CreateThread( workerthread );
    handles[3] = CreateThread( workerthread );
    handles[4] = CreateThread( workerthread );
    handles[5] = CreateThread( workerthread );
    handles[6] = CreateThread( workerthread );
    handles[7] = CreateThread( workerthread );

    Sleep( 5000 ); //wait 5 seconds before writing to it
    closeworkerthreads = true; //manager thread decides all worker threads can close

    WaitForMultipleObjects( handles ); //Wait till all threads are closed
}

Show me with good examples that I'm wrong, please.