Feb 9, 2011

Debug the Code : Thread synchronization

The program is supposed to print multiples of 10 in a new line in increasing order. Although, the program appears to be correct, it is not. It is giving me duplicate values in the output. Locate the problem?

#include <windows.h>
#include <process.h>
#include <iostream>
#include <assert.h>
#include "Lock.h"

using namespace examples;

static bool alive = true;
static int current = 0;

unsigned __stdcall put(void *args)
{
    Lock lock;
    while(alive)
    {
        lock.acquire();
        current = current + 10;
        ::Sleep(500);
        std::cout << current << std::endl;
        lock.release();
    }
    return 0;
}

int main()
{
    // create threads
    unsigned t1;
    HANDLE h1 = (HANDLE) ::_beginthreadex(0, 0, &put, 0, CREATE_SUSPENDED, &t1);
    assert(h1 != 0);

    unsigned t2;
    HANDLE h2 = (HANDLE) ::_beginthreadex(0, 0, &put, 0, CREATE_SUSPENDED, &t2);
    assert(h2 != 0);

    // start threads
    ::ResumeThread(h1);
    ::ResumeThread(h2);

    ::Sleep(10000);
    alive = false;

    ::WaitForSingleObjectEx(h1, INFINITE, false);
    ::WaitForSingleObjectEx(h2, INFINITE, false);

    ::CloseHandle(h1);
    ::CloseHandle(h2);

    return 0;
}

Answer

The two threads are created with two different instances of Lock. Hence, there is actually no synchronization happening at all. The fix is to change the Lock type to static or make it global
unsigned __stdcall put(void *args)
{
    static Lock lock;
    while(alive)
    {
        lock.acquire();
        current = current + 10;
        ::Sleep(500);
        std::cout << current << std::endl;
        lock.release();
    }
    return 0;
}
Output after the fix:

No comments :

Post a Comment