CODEWITHSUNDEEP
c++cmultithreading
chacham15
chacham15

Reputation: 14281

What is the race condition in this code?

The following is my implementation of SleepConditionVariableCS, WakeAllConditionVariable, and the thread creation functions. The problem is that sometimes when I try to create a thread the creating thread gets stuck on WaitForSingleObject( cv->mut, INFINITE ); in SleepConditionVariableCS. I cant figure out what the race condition here is. 


typedef struct
{
  int waiters_count;
  HANDLE sema_;
  HANDLE mut;
} CONDITION_VARIABLE;

void SleepConditionVariableCS(CONDITION_VARIABLE *cv, CRITICAL_SECTION *cs, int32_t dwMilliseconds){
    WaitForSingleObject( cv->mut, INFINITE ); //Acuire object lock

    cv->waiters_count++;
    LeaveCriticalSection (cs);
    if (SignalObjectAndWait(cv->mut, cv->sema_, dwMilliseconds, FALSE) == WAIT_TIMEOUT){ //SignalObjectAndWait releases the lock
        cv->waiters_count--;
    }
    EnterCriticalSection(cs);
}

void WakeAllConditionVariable(CONDITION_VARIABLE *cv){
    WaitForSingleObject( cv->mut, INFINITE );

    while (cv->waiters_count > 0){
        cv->waiters_count = cv->waiters_count - 1;
        ReleaseSemaphore (cv->sema_, 1, 0);
    }
    ReleaseMutex(cv->mut);
}

void KernelThread_CreationWait(void *kthread){
    KernelThread *thread =  (KernelThread *) kthread;

    EnterCriticalSection(thread->lock);
    thread->state = Thread_CREATED;

    WakeAllConditionVariable(thread->condition_variable);

    LeaveCriticalSection(thread->lock);
    KernelThread_main(kthread);
}

KernelThread* createKernelThread(){
    EventHandler_getHandler();

    unsigned long threadid;
    int t;
    void *hand;

    KernelThread *thread = KernelThread_malloc();
    EnterCriticalSection(thread->lock);
    thread->state = Thread_WAITINGFORCREATION;
    hand = CreateThread(NULL,
            0, // security, stack size
            (LPTHREAD_START_ROUTINE)&KernelThread_CreationWait, // start
            (void *)thread,
            0,
            &threadid); // param, creation flags, id
    if (hand == NULL){
        printf("ERROR: return handle from CreateThread() is NULL\n");
        exit(-1);
    }
    thread->thread = hand;
    thread->thread_id = threadid;

    SleepConditionVariableCS(thread->condition_variable,thread->lock,INFINITE);
    LeaveCriticalSection(thread->lock);

    return thread;
}

void InitializeConditionVariable (CONDITION_VARIABLE *cv){
    cv->waiters_count = 0;
    cv->sema_ = CreateSemaphore (NULL,       // no security
                                0,          // initially 0
                                0x7fffffff, // max count
                                NULL);      // unnamed
    cv->mut = CreateMutex(
          NULL,              // default security attributes
          FALSE,             // initially not owned
          NULL);             // unnamed mutex
}

Upvotes: 1

Views: 970

Answers (2)

Wandering Logic
Wandering Logic

Reputation: 3403

Check out the following very detailed analysis of the gotchas when trying to implement pthread-style condition variables using win32 apis. http://www.cs.wustl.edu/~schmidt/win32-cv-1.html. (Section 3.4, in particular).

In your code I see only one problem at the moment. In SleepConditionVariableCS():

    LeaveCriticalSection (cs);
    if (SignalObjectAndWait(cv->mut, cv->sema_, dwMilliseconds, FALSE) == WAIT_TIMEOUT){ //SignalObjectAndWait releases the lock
        // RIGHT HERE YOU HAVE NOT ACQUIRED cv->mut
        cv->waiters_count--; // SO THIS IS A DATA RACE (with the cv->waiters_count uses in WakeAllConditionVariable()
    }

Upvotes: 1

JimR
JimR

Reputation: 16153

I see no logic flaws that really stand out to me.

However...

Do you initialize CONDITION_VARIABLE to a known state before using it?

You do not check the return from a few of the system calls which could be returning an error that you'll never see. WaitForSingleObject and EnterCriticalSection can fail or be given a bad handle.

Do you always lock thread->lock and keep it locked while dealing with the condition variables? I see 2 different pieces of code with a reference to thread->lock.

Do you lock/unlock your critical sections and sems in the same order every time?

Upvotes: 0

Related Questions