Critical Section for a few threads

Question

I'm trying to understand mutex, semaphores and critical section, and I've got a problem with the last one. I wrote a little program which prints numbers in separate threads, but I want them to print 5 numbers in row without interference from other threads.

For that purpose I used the structure CRITICAL_SECTION and its functions. I initialize critical section in main thread (and delete it when finished), enter to crit section in the cycle where I'm printing my numbers and trying to release crit section when I reach my condition. As I understand, when I do this action, the other thread can be started and locked for printing 5 values but it can also be same thread.

I expect to see this:

Thread 1: 0
Thread 1: 1
Thread 1: 2
Thread 1: 3
Thread 1: 4
Thread 2: 0
Thread 2: 1
Thread 2: 2
Thread 2: 3
Thread 2: 4
Thread 3: 0
Thread 3: 1
Thread 3: 2
Thread 3: 3
Thread 3: 4
Thread 2: 5
Thread 2: 6
Thread 2: 7
Thread 2: 8
Thread 2: 9

but for now I see only one thread printing 0-9 values and that's all. What did I do wrong?

Here is my code:

#include <stdio.h>
#include <conio.h>
#include <windows.h>

#define THREAD_NUM          3
#define BASE_THREAD_NUM     1
#define COUNT               10

CRITICAL_SECTION critsection;

typedef struct DATA_ {
    INT threadNum;
    INT data[COUNT];
} DATA, *PDATA;

DWORD WINAPI PrintValsThread(LPVOID lpParam)
{
    DATA* pVal = (DATA*)lpParam;
    INT counter = 0;

    for (USHORT i = 0; i < COUNT; ++i) {
        if ((counter % 5) == 0)
            EnterCriticalSection(&critsection);

        pVal->data[i] = i;
        printf("Thread %d, i = %d\n", pVal->threadNum, pVal->data[i], counter);
        counter++;

        if ((counter % 5) == 0 && counter > 0)
            LeaveCriticalSection(&critsection);
    }
    return 0;
}

INT main(INT argc, WCHAR **argv)
{
    DATA* pVal[THREAD_NUM + 1];
    DWORD dwthreadNumArray[THREAD_NUM];
    HANDLE hThreadArray[THREAD_NUM];

    InitializeCriticalSection(&critsection);
    for (INT i = 0; i < THREAD_NUM; ++i) {
        pVal[i] = (DATA*)malloc(sizeof(DATA));
        if (pVal[i] == NULL) {
            printf("pVal[%d] == NULL\n", i);
            break;
        }
        pVal[i]->threadNum = BASE_THREAD_NUM + i;

        hThreadArray[i] = CreateThread(NULL, 0, PrintValsThread, pVal[i], 0, &dwthreadNumArray[i]);
        if (hThreadArray[i] == NULL) {
            printf("hThreadArray[%d] failed, error %d\n", i, GetLastError());
        break;
        }

    }

    INT wait = WaitForMultipleObjects(THREAD_NUM, hThreadArray, TRUE, INFINITE);
    if (wait == WAIT_FAILED)
        printf("The function WaitForMultipleObjects() has been failed\n");

    for (INT i = 0; i < THREAD_NUM; ++i) {
        if (pVal[i] != NULL)
            free(pVal[i]);
        CloseHandle(hThreadArray[i]);
    }

    DeleteCriticalSection(&critsection);
    printf("It's over\n");

    _getch();
    return 0;
}

Oh, I see, you are trying to lock out all the other threads for five values. Unfortunately, PrintValsThread() is borken. You are entering the CS every time tound the loop, but only leaving it on the '((counter % 5) == 0 && counter > 0)' condition. Fail. Redesign your function so that it works. — Martin James
Ah, now I see the mistake, thank you. Iadded this condition inside the cycle and it worked as I wanted: if ((counter % 5) == 0) EnterCriticalSection(&critsection); Thank you! Changed the code — ezviagintcev

ezviagintcev ezviagintcev · Accepted Answer · 2018-04-01T17:00:51

This is the solution of the problem:

for (USHORT i = 0; i < COUNT; ++i) {
    if ((counter % 5) == 0)
        EnterCriticalSection(&critsection);

    pVal->data[i] = i;
    printf("Thread %d, i = %d\n", pVal->threadNum, pVal->data[i], counter);
    counter++;

    if ((counter % 5) == 0 && counter > 0)
        LeaveCriticalSection(&critsection);
}

Critical Section for a few threads

1 Answers