threading question: is this code going to work perfectly?

[Abubakar]

Hi,

I created 2 functions which run in seperate threads. what they do simply is
that thread1 starts and does something while the thread 2 waits for it. Then
after thread1 is done, it resumes the second thread by calling SetEvent and
itself goes into wait condition by calling WaitForSingleObject(). Now Thread
2 does something and when its done it resumes the thread1 and goes to wait
itself. This goes on for a variable number of time than ends. For this I
wrote a sample code that does this and I'm pasting it over here. My question
is that can anyone tell me that in the following code are there any chances
of the synchronisation getting disturbed? Can this happen in the following
code or is it just perfectly written (from threading point of view)? By
synchronisation getting disturbed I mean thread1 doing what it does more
than one 1 time without thread2 doing its work.

You can also assume that a lot of other threads will also be getting
executed in the process, like some thread getting data from the database,
some thread establishing socket connections and doing communication there,
some threads updating GUI etc ...

And plz also tell me that I'm even writing this code properly or r there any
fundamental problems in there.

I wrote the following code:
//==========================================
// cs.cpp : Defines the entry point for the console application.
//
#include "stdafx.h"
#include <windows.h>
#include <iostream>
using namespace std;

HANDLE thread1wait, thread2wait;
int a,b, t1count = 0, t2count = 0, looplimit = 100; //looplimit can be in
millions
DWORD WINAPI thread1 (LPVOID param)
{

::Sleep(2000);
cout<<"now..."<<endl;
while (t1count++ < looplimit)
{
if (a == 0 && b == 0)
:ebugBreak();

int z = a+ b;
if (z!= 13)
cout<<"not 13 !"<<endl;

a = b = 0;
::SetEvent (thread2wait);
::WaitForSingleObject (thread1wait, INFINITE);
}
cout<<"thread 1 count is "<<t1count<<endl;
return NULL;
}
DWORD WINAPI thread2 (LPVOID param)
{
while (t2count++ < looplimit)
{
::WaitForSingleObject (thread2wait, INFINITE);
a = 4; b = 9;
::SetEvent (thread1wait );
}
cout<<"thread 2 count is "<<t2count<<endl;
return NULL;
}

int _tmain(int argc, _TCHAR* argv[])
{
a = 4; b = 9;
cout<<"hello"<<endl;

HANDLE thid1, thid2;
DWORD junk;
thread1wait = ::CreateEventA(NULL, FALSE, FALSE, NULL);
thread2wait = ::CreateEventA(NULL, FALSE, FALSE, NULL);
thid1 = ::CreateThread (NULL, NULL, thread1, NULL, NULL, &junk);
thid2 = ::CreateThread (NULL, NULL, thread2, NULL, NULL, &junk);
HANDLE handles [] = {thid1, thid2};
::WaitForMultipleObjects(2, handles, TRUE, INFINITE);
return 0;
}

//==========================================

Regards,

...ab

 

[Tom Widmer [VC++ MVP]]

Hi,

I created 2 functions which run in seperate threads. what they do simply is
that thread1 starts and does something while the thread 2 waits for it. Then
after thread1 is done, it resumes the second thread by calling SetEvent and
itself goes into wait condition by calling WaitForSingleObject(). Now Thread
2 does something and when its done it resumes the thread1 and goes to wait
itself. This goes on for a variable number of time than ends.

Sounds like a job for one thread to me. If you have two threads and can
only have one of them running at a time, the work could be done in a
single thread by restructuring the code (though in some cases the
restructuring is more complicated than it is worth).

For this I

wrote a sample code that does this and I'm pasting it over here. My question
is that can anyone tell me that in the following code are there any chances
of the synchronisation getting disturbed? Can this happen in the following
code or is it just perfectly written (from threading point of view)? By
synchronisation getting disturbed I mean thread1 doing what it does more
than one 1 time without thread2 doing its work.

The general technique used should work fine. It can be improved though
in a way which might prevent unnecessary context switches - see below:

DWORD WINAPI thread1 (LPVOID param)
{

::Sleep(2000);
cout<<"now..."<<endl;
while (t1count++ < looplimit)
{
if (a == 0 && b == 0)
:ebugBreak();

int z = a+ b;
if (z!= 13)
cout<<"not 13 !"<<endl;

a = b = 0;
::SetEvent (thread2wait);
::WaitForSingleObject (thread1wait, INFINITE);

The above two lines can be rewritten as:
SignalObjectAndWait(thread2wait, thread1wait, INFINITE, FALSE);

}
cout<<"thread 1 count is "<<t1count<<endl;
return NULL;
}
DWORD WINAPI thread2 (LPVOID param)
{
while (t2count++ < looplimit)
{
::WaitForSingleObject (thread2wait, INFINITE);
a = 4; b = 9;
::SetEvent (thread1wait );
}

That could be:
if (t2count < looplimit)
::WaitForSingleObject (thread2wait, INFINITE);

while (t2count++ < looplimit)
{
a = 4; b = 9;
if (tcount < looplimit)
SignalObjectAndWait(thread1wait, thread2wait, INFINITE, FALSE);
else
SetEvent(thread1wait);
}

to avoid unnecessary context switches.

cout<<"thread 2 count is "<<t2count<<endl;

Note that cout is not threadsafe - you should protect calls with a
critical section or mutex.

Tom

 

[Abubakar]

The above two lines can be rewritten as:

SignalObjectAndWait(thread2wait, thread1wait, INFINITE, FALSE);

I think SignalObjectAndWait is what I needed.

Thanks,

...ab

 

[Ben Voigt]

Sounds like a job for one thread to me. If you have two threads and can
only have one of them running at a time, the work could be done in a
single thread by restructuring the code (though in some cases the
restructuring is more complicated than it is worth).

Fibers might be a better match than threads for this scenario, while
maintaining the synchronous code flow.