threading question: is this code going to work perfectly?

  • Thread starter Thread starter Abubakar
  • Start date Start date
A

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)
::DebugBreak();

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
 
Abubakar said:
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.
Click to expand...

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.
Click to expand...

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)
::DebugBreak();

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

a = b = 0;
::SetEvent (thread2wait);
::WaitForSingleObject (thread1wait, INFINITE);
Click to expand...

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 );
}
Click to expand...

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;
Click to expand...

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

Tom
 
The above two lines can be rewritten as:
SignalObjectAndWait(thread2wait, thread1wait, INFINITE, FALSE);
Click to expand...

I think SignalObjectAndWait is what I needed.

Thanks,

...ab
 
Tom Widmer said:
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).
Click to expand...


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