Thread question

[JMJAtlanta]

I'm pondering a question, and have made a harness to do some testing, and would like someone who is smarter than me to straighten my head out.

Here's the important section of code:

long GetNextNumber() {
	long retVal = 0;
	myMutex.lock();
	if (currentNumber == LONG_MAX)
		currentNumber = 0;
	retVal = ++currentNumber;
	myMutex.unlock();
	return retVal;
}

With lines 3 and 7 commented out, I can have 2 threads get the same number from this method. With the code as-is above, I can't get the same number from this method, at least in my testing.

Scenario: Thread A running through line 7 then Thread B running all lines, then Thread A running line 8.

Questions:
Is there something stopping this from happening? Or perhaps I haven't run enough cycles to get this (probably rare) scenario to happen?

If this scenario can happen, how would you prevent it?

Here's my full code in case you want to play. I'm running this in Fedora 17 in a VMWare virtual machine. The scenario hasn't happened (yet) in that environment.

/*
 * main.cpp
 *
 *  Created on: Aug 10, 2012
 *      Author: jmjatlanta
 */

#include <thread>
#include <iostream>
#include <chrono>
#include <mutex>
#include "limits.h"

using namespace std;

class NumberQueue {
private:
	long currentNumber;
	mutex myMutex;
public:
	NumberQueue() { currentNumber = 0;}
	~NumberQueue() {}
	long GetNextNumber() {
		long retVal = 0;
		//myMutex.lock();
		if (currentNumber == LONG_MAX)
			currentNumber = 0;
		retVal = ++currentNumber;
		//myMutex.unlock();
		return retVal;
	}
};

class NumberGrabber {
private:
	shared_ptr<NumberQueue> numberQueue;
public:
	shared_ptr<NumberGrabber> Sibling;
	long MyNumber;
	bool Run;
	NumberGrabber(shared_ptr<NumberQueue> queue) : MyNumber(-1), Run(true), numberQueue(queue) { }
	~NumberGrabber() {}
	void RunOnMyOwn() {
		while(Run) {
			MyNumber = (*numberQueue).GetNextNumber();
			long theirNumber = (*Sibling).MyNumber;
			if (MyNumber == theirNumber) {
				cout << "My Number [" << MyNumber << "] is the same as my sibling [" << theirNumber << "]" << endl;
			}
		}
	}
};

void runThread(shared_ptr<NumberGrabber> grabber) {
	(*grabber).RunOnMyOwn();
}

int main(int argc, char* argv[]) {
	shared_ptr<NumberQueue> numberQueue(new NumberQueue());
	shared_ptr<NumberGrabber> grabber1(new NumberGrabber(numberQueue));
	shared_ptr<NumberGrabber> grabber2(new NumberGrabber(numberQueue));

	(*grabber1).Sibling = grabber2;
	(*grabber2).Sibling = grabber1;

	cout << "Firing up thread 1" << endl;
	thread t1(runThread, grabber1);
	cout << "Firing up thread 2" << endl;
	thread t2(runThread, grabber2);
	cout << "Pausing main thread by spinning" << endl;
	for(long l = 0; l < LONG_MAX; l++) {
		// something that takes time
		this_thread::sleep_for(chrono::milliseconds(100));
	}
	(*grabber1).Run = false;
	(*grabber2).Run = false;

	cout << "About to join thread 2" << endl;
	t2.join();
	cout << "About to join thread 1" << endl;
	t1.join();
	cout << "Ending the program" << endl;
	return 0;
}

[Cubbi]

Thread A running through line 7 then Thread B running all lines, then Thread A running line 8.

Questions:
Is there something stopping this from happening?

Nothing: the code, written as-is, may indeed result in thread B receiving the number 10, then thread A receiving the number 9, for example (which was sitting in A's own retVal at line 7 while B was running the whole function call).

What it *is* preventing, is two threads receiving the same number.

[coder777]

Questions:
Is there something stopping this from happening? Or perhaps I haven't run enough cycles to get this (probably rare) scenario to happen?

Nothing bad will happen. retVal is a local variable hence different for each caller.

[Peter87]

retVal is a local variable so each call to the function will contain it's own copy of retVal. It's not possible for one thread to access retVal in the other thread so it is safe as the way you have it.

[JMJAtlanta]

Okay, after some more digging, I think I found out why. This article helped:

http://pic.dhe.ibm.com/infocenter/aix/v7r1/index.jsp?topic=%2Fcom.ibm.aix.genprogc%2Fdoc%2Fgenprogc%2Fwriting_reentrant_thread_safe_code.htm

and it has to do with where the method's variables are kept. Local variables (i.e. retVal) are on the stack (not shared by threads), where as currentNumber is in shared memory space.

Is my thinking correct?

[JMJAtlanta]

Hehe. You guys are fast. I should have waited a few more seconds before digging. Thanks!