CODEWITHSUNDEEP
c++windowswinapiprocesssemaphore
Andrian Babii
Andrian Babii

Reputation: 1

Windows semaphores

I need to implement a program which uses a semaphore to limit the number of opened notepad windows to 10. I created a semaphore in WM_INITDIALOG 

    semafor = CreateSemaphore(0, 1, 10, "semaphore");

and every time I click on button to open a new window, I open that semaphore. However it doesn't stop me from opening more than 10 windows.

Here is my code when I click on button in a dialog box to open a new window:

case WM_COMMAND:
    switch (LOWORD(wParam)) {
        case ID_OK: 

            semafor = OpenSemaphore(SEMAPHORE_ALL_ACCESS, TRUE, "semaphore");
            if (semafor == NULL) {
                printf("Eroare deschidere semafor empty: %d \n", GetLastError());
                ExitProcess(1);
            }

            BOOL b = CreateProcess("C:\\Windows\\System32\\notepad.exe",
                NULL, NULL, NULL, TRUE, 0, NULL, NULL,
                &si, &pi);

            process[++i] = GetCurrentProcess();

            if (b) {
                dwWaitForChild = WaitForInputIdle(pi.hProcess, 2000);
                switch (dwWaitForChild) {
                case 0:
                    printf("Procesul fiu este ready!\n");
                    break;
                case WAIT_TIMEOUT:
                    printf("Au trecut 2 sec. si procesul fiu nu este ready!\n");
                    break;
                case 0xFFFFFFFF:
                    printf("Eroare!\n");
                    break;
                }

                WaitForMultipleObjects(i, process, TRUE, INFINITE);

                iRasp = MessageBox(NULL, "Terminam procesul fiu?", "Atentie!", MB_YESNO);
                if (iRasp == IDYES) {
                    if (TerminateProcess(pi.hProcess, 2)) {

                        DWORD dwP;

                        GetExitCodeProcess(pi.hProcess, &dwP);
                        printf("Codul de terminare al procesului fiu: %d\n", dwP);
                        ReleaseSemaphore(semafor, 1, NULL);
                        CloseHandle(pi.hProcess);
                        printf("\nProcesul fiu a fost terminat cu succes\n");
                    }
                    else {
                        //tiparim mesajul de eroare 
                        TCHAR buffer[80];
                        LPVOID lpMsgBuf;
                        DWORD dw = GetLastError();

                        FormatMessage(
                            FORMAT_MESSAGE_ALLOCATE_BUFFER |
                            FORMAT_MESSAGE_FROM_SYSTEM,
                            NULL,
                            dw,
                            MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
                            (LPTSTR)&lpMsgBuf,
                            0, NULL);

                        wsprintf(buffer, "TerminateProcess() a esuat cu eroarea %d: %s",
                            dw, lpMsgBuf);

                        MessageBox(NULL, buffer, "Eroare!", MB_OK);

                        LocalFree(lpMsgBuf);
                    }
                } // rasp YES

            }
            else
                printf("Eroare la crearea procesului fiu!\n");

            return TRUE;
        }
        break;
    }
    return FALSE;
}

Upvotes: 0

Views: 6213

Answers (2)

Remy Lebeau
Remy Lebeau

Reputation: 596256

You are misusing the semaphore. Create the semaphore one time and reuse it, don't re-open it over and over. But, more importantly, you have to wait on the semaphore, such as with WaitForSingleObject(), to decrement its counter, and then release it with ReleaseSeamphore() to increment its counter. The state of the semaphore is signaled when its counter is greater than 0, and unsignaled when 0.

So, you need to create the semaphore with an initial count of 10 and then wait on it each time you want to create a new process. If the wait fails, too many processes are already running. Otherwise, if the wait succeeds, the counter has been decremented, so create the new process, and then increment the counter when the process ends.

However, that being said, your semaphore is completely useless in the context of the code you showed. You want to use the semaphore to control the number of running instances of Notepad, but as soon as you launch 1 instance, you are attempting (incorrectly, I might add) to block your code until that instance exits. So you will never be able to run more than 1 instance at a time, making the semaphore useless. The fact that you are able to run more than 1 is likely due to a bug in your code (you are waiting on the wrong process handles!). Don't use a blocking wait at all. Let the system notify you when each spawned process ends.

A correct way to use a semaphore in your code would look something more like this instead:

const UINT WM_PROCESS_ENDED = WM_APP + 1;
const int MAX_PROCESSES = 10;

typedef struct procInfo
{
    HANDLE hProcess;
    HANDLE hWait;
} procInfo;

HANDLE semafor = NULL;
procInfo process[MAX_PROCESSES] = {};
int numProcesses = 0;
HWND hwndDialog;

...

VOID CALLBACK ProcessEnded(PVOID lpParameter, BOOLEAN TimerOrWaitFired)
{
    PostMessage(hwndDialog, WM_PROCESS_ENDED, 0, (LPARAM)lpParameter);
}

...

case WM_INITDIALOG:
{
    hwndDialog = hwnd;

    semafor = CreateSemaphore(0, MAX_PROCESSES, MAX_PROCESSES, NULL);
    if (semafor == NULL)
    {
        printf("Eroare deschidere semafor empty: %d \n", GetLastError());
        ExitProcess(1);
    }

    break;
}

case WM_DESTROY:
{
    for (int i = 0; i < numProcesses; ++i)
    {
        UnregisterWaitEx(process[i].hWait, INVALID_HANDLE_VALUE);
        TerminateProcess(process[i].hProcess, 2);
        CloseHandle(process[i].hProcess);
    }

    CloseHandle(semafor);
    semafor = NULL;

    hwndDialog = NULL;

    break;
}

case WM_COMMAND:
    switch (LOWORD(wParam))
    {
        case ID_OK:
        {
            if (WaitForSingleObject(semafor, 0) != WAIT_OBJECT_0)
            {
                // too many instances running...
                break;
            }

            if (!CreateProcess("C:\\Windows\\System32\\notepad.exe",
                NULL, NULL, NULL, TRUE, 0, NULL, NULL,
                &si, &pi))
            {
                printf("Eroare la crearea procesului fiu!\n");
                ReleaseSemaphore(semafor, 1, NULL);
                break;
            }

            CloseHandle(pi.hThread);

            dwWaitForChild = WaitForInputIdle(pi.hProcess, 2000);
            switch (dwWaitForChild)
            {
                case 0:
                    printf("Procesul fiu este ready!\n");
                    break;

                case WAIT_TIMEOUT:
                    printf("Au trecut 2 sec. si procesul fiu nu este ready!\n");
                    break;

                case WAIT_FAILED:
                    printf("Eroare!\n");
                    break;
            }

            procInfo info;

            info.hProcess = pi.hProcess;
            if (!RegisterWaitForSingleObject(&info.hWait, pi.hProcess, &ProcessEnded, pi.hProcess, INFINITE, WT_EXECUTELONGFUNCTION | WT_EXECUTEONLYONCE))
            {
                TerminateProcess(pi.hProcess, 2);
                ReleaseSemaphore(semafor, 1, NULL);
                break;
            }

            process[numProcesses++] = info; 
        }
    }

    break;
}

case WM_PROCESS_ENDED:
{
    HANDLE hProcess = (HANDLE)lParam;

    for (int i = 0; i < numProcesses; ++i)
    {
        if (process[i].hProcess == hProcess)
        {
            UnregisterWait(process[i].hWait);

            for (int j = i + 1; j < numProcesses; ++j)
                process[j-1] = process[j];

            --numProcesses;

            break;
        }
    }

    CloseHandle(hProcess);
    ReleaseSemaphore(semafor, 1, NULL);
}

In which case, you can just get rid of the semaphore altogether since you already have your own counter:

const UINT WM_PROCESS_ENDED = WM_APP + 1;
const int MAX_PROCESSES = 10;

typedef struct procInfo
{
    HANDLE hProcess;
    HANDLE hWait;
} procInfo;

procInfo process[MAX_PROCESSES] = {};
int numProcesses = 0;
HWND hwndDialog;

...

VOID CALLBACK ProcessEnded(PVOID lpParameter, BOOLEAN TimerOrWaitFired)
{
    PostMessage(hwndDialog, WM_PROCESS_ENDED, 0, (LPARAM)lpParameter);
}

...

case WM_INITDIALOG:
{
    hwndDialog = hwnd;
    break;
}

case WM_DESTROY:
{
    for (int i = 0; i < numProcesses; ++i)
    {
        UnregisterWaitEx(wait[i], INVALID_HANDLE_VALUE);
        TerminateProcess(process[i], 2);
        CloseHandle(process[i]);
    }

    hwndDialog = NULL;

    break;
}

case WM_COMMAND:
    switch (LOWORD(wParam))
    {
        case ID_OK:
        {
            if (numProcesses >= MAX_PROCESSES)
            {
                // too many instances running...
                break;
            }

            if (!CreateProcess("C:\\Windows\\System32\\notepad.exe",
                NULL, NULL, NULL, TRUE, 0, NULL, NULL,
                &si, &pi))
            {
                printf("Eroare la crearea procesului fiu!\n");
                break;
            }

            CloseHandle(pi.hThread);

            dwWaitForChild = WaitForInputIdle(pi.hProcess, 2000);
            switch (dwWaitForChild)
            {
                case 0:
                    printf("Procesul fiu este ready!\n");
                    break;

                case WAIT_TIMEOUT:
                    printf("Au trecut 2 sec. si procesul fiu nu este ready!\n");
                    break;

                case WAIT_FAILED:
                    printf("Eroare!\n");
                    break;
            }

            procInfo info;

            info.hProcess = pi.hProcess;
            if (!RegisterWaitForSingleObject(&info.hWait, pi.hProcess, &ProcessEnded, pi.hProcess, INFINITE, WT_EXECUTELONGFUNCTION | WT_EXECUTEONLYONCE))
            {
                TerminateProcess(pi.hProcess, 2);
                break;
            }

            process[numProcesses++] = info; 
        }
    }

    break;
}

case WM_PROCESS_ENDED:
{
    HANDLE hProcess = (HANDLE)lParam;

    for (int i = 0; i < numProcesses; ++i)
    {
        if (process[i].hProcess == hProcess)
        {
            UnregisterWait(process[i].hWait);

            for (int j = i + 1; j < numProcesses; ++j)
                process[j-1] = process[j];

            --numProcesses;

            break;
        }
    }

    CloseHandle(hProcess);
}

Upvotes: 0

crapier
crapier

Reputation: 373

You aren't ever waiting (or P in semaphore logic) on the semaphore.

Check the winapi example on how to use a semaphore. In their example they use WaitForSingleObject. You are only using OpenSemaphore which just gives you the semaphore handle so you can use it in multiple processes, you then need to actually wait on it if you want to decrement its value and wait/timeout when it's 0.

You should be doing something like this after you open your semaphore and before trying to open the notepad instance:

// Try to enter the semaphore gate.

dwWaitResult = WaitForSingleObject( 
    ghSemaphore,   // handle to semaphore
    0L);           // zero-second time-out interval

Upvotes: 3

Related Questions