Mutex and Multiple Threads Problem

[roweboats]

I am in school and have been given the problem to use multiple threads and mutex to get two random numbers and in the second thread, produce the greates common factor. And, I need to have this happen 10 times. I have tried and was successful in creating most of the problem but when I attempt to use a mutex, it runs thread 1 (main function) twice before releasing the mutex. Then it runs thread two 7 times before going back to thread 1 for two more times then the last 3 times for thread 2 and finally the remaining times in thread 1. I've tried setting the second parameter in the creation of the mutex to true and thread 2 never runs at all. any help would be appreciated.

[roweboats]

#include <process.h>
#include <windows.h>
#include <stdio.h>
#include <time.h>

int randNum1 = 0;
int randNum2 = 0;
int greatCommonDivisor = 0;
HANDLE hThread1;
unsigned threadID1;
HANDLE hThread2;
unsigned threadID2;
HANDLE ghMutex;

int euclidGCD( int a, int b )
  {
  if (b == 0)
    return a;
  else
    return euclidGCD( b, a % b );
  }

unsigned __stdcall SecondThreadFunc( void* pArguments ) 
{
	DWORD dwWaitResult2;
	for(int indx = 0; indx < 10; indx++)
	{
		dwWaitResult2 = WaitForSingleObject(
		ghMutex,
		5000L); 
		switch (dwWaitResult2)
		{
			case WAIT_OBJECT_0:
			__try 
			{
				printf( "In second thread...\n" );
				greatCommonDivisor = euclidGCD(randNum1, randNum2);
				printf( "Greatest common divisor is  %d\n", greatCommonDivisor  );
				
			}
			__finally 
			{
				// Release ownership of the mutex object.
				if (! ReleaseMutex(ghMutex))
				{
					// Deal with error.
				}
				break;
			}
			case WAIT_TIMEOUT:
				return FALSE;
			case WAIT_ABANDONED:
				return FALSE;
		}
	}
	return 0;
}


void main()
{
	// initialize random seed: 
	// srand ( time(NULL) );		// use after design and testing
	srand ( 4 );					// used for design and testing

	// Create a mutex with no initial owner
    ghMutex = CreateMutex( 
        NULL,						// default security attributes
        FALSE,						// initially not owned
        NULL);						// unnamed mutex

	printf( "Creating second thread...\n" );
	
	// Create the second thread.
	hThread2 = (HANDLE)_beginthreadex( NULL, 0, &SecondThreadFunc, NULL, 0,
	&threadID2 );
	
	DWORD dwWaitResult;
     for(int i = 0; i < 10; i++)
	{
		// Request ownership of mutex.
		dwWaitResult = WaitForSingleObject(
		ghMutex,						
		5000L);							

		switch (dwWaitResult)
		{
			// The thread got mutex ownership.
			case WAIT_OBJECT_0:
			__try 
			{
				printf( "In first thread...\n" );
		
				/* generate random numbers: */
				randNum1 = rand() % 100 + 1;
				randNum2 = rand() % 100 + 1;
		
				printf( "Random number one is  %d\n", randNum1 );
				printf( "Random number two is  %d\n", randNum2 );
				
			}
			__finally 
			{
				// Release ownership of the mutex object.
				if (! ReleaseMutex(ghMutex))
				{
				}
				break;
			}
			case WAIT_TIMEOUT:
				return;
			case WAIT_ABANDONED:
				return;
		}
	}	

	_endthreadex( 0 );
	// Destroy the thread object.
//	CloseHandle( hThread1 );
	CloseHandle( hThread2 );
}

[syzygy]

I'm not 100% sure, but I think your problem is that the thread that has the mutex locked can release the mutex, then it goes back to the top of the for loop, sees the mutex is unlocked and then locks it again for itself before the other thread has a chance to lock it. Basically, you've created a race condition between the 2 threads for who gets to lock the mutex after it's been released.

Edit: Actually, I must be wrong, cause your functions return if WaitForSingleObject times out, but you're not seeing that. Still, I would change the timeout value to INFINITE, just to be sure. Also, put at least a print in your ReleaseMutex() block so you know for sure it's not failing.

[roweboats]

That didn't work, but i tried a Sleep(.1); in before the release of the mutex in both places and it works. I was hoping not to do this but I can probably get away with it.

[Codeplug]

There's no guarantee of any particular interleaving - with or without sleep.

For a single consumer and single producer, you only need 2 auto-reset Events: "try to consume" (TTC) and "try to produce" (TTP).
The producer can't produce until TTP is signaled by the consumer.
The consumer can't consume until TTC is signaled by the producer.

In general, you want to minimize the amount of work that's done while holding a lock. In the original code, that would mean reading and writing randNumN to/from local variables only while the lock is held.
In the 2-event model, the producer thread would pre-compute the next randNumN so that they are available immediately when TTP is signaled.

gg

[roweboats]

There's no guarantee of any particular interleaving - with or without sleep.

For a single consumer and single producer, you only need 2 auto-reset Events: "try to consume" (TTC) and "try to produce" (TTP).
The producer can't produce until TTP is signaled by the consumer.
The consumer can't consume until TTC is signaled by the producer.

In general, you want to minimize the amount of work that's done while holding a lock. In the original code, that would mean reading and writing randNumN to/from local variables only while the lock is held.
In the 2-event model, the producer thread would pre-compute the next randNumN so that they are available immediately when TTP is signaled.

gg

So are you saying there is no way to use a mutex as I have done to ensure 1 happens before the other?

[syzygy]

I believe Codeplug is saying that you need some kind of flagging system so that each thread knows when to perform its duty. You can't rely on whether the mutex is locked or unlocked as a flagging mechanism because you cannot control when each thread will perform a check on the mutex, and therefore cannot guarantee interleaved performance.

[roweboats]

thanks all.

[Codeplug]

>> So are you saying there is no way to use a mutex as I have done to ensure 1 happens before the other?
You could use a mutex which also protects two boolean flags: TTP and TTC.
But then the threads would have to poll - which is inefficient and should be avoided if possible.

gg