QMutex seems not to lock()

[sylvaticus]

Hello.. I am in the progress of parallelise a multi-agent model where "agents" are different instances (thousands) of a class and they have to independently perform a relatively cpu-intensive task (few seconds).
Looks the perfect situation for a reentrant code .... it seems...
My problem is that, even if I mutex.lock() all the should-be-concurrent code (for debugging reasons of course) my application crash, while if I use a single thread it works.
This is the code that I implemented so far:

void
Manager_farmers::landAllocation(){
	// .. serial code ..
 
	for (int i=0; i < nThreads; i++){
		Manager_farmers_threads* wThread = new Manager_farmers_threads;
		wThreads.push_back(wThread);
	}
 
	// parallelise the code to z workingThreads.. 
	for (uint y=0;y<myAgents.size();y++){
		bool assigned = false;
		while(!assigned) {
			for (uint z=0;z<wThreads.size();z++){
			  if (!wThreads[z]->isRunning()){
				wThreads[z]->assignJob(myAgents[y], freePlot);
				wThreads[z]->start();
				assigned = true;
				break;
			  }
			}
		}	
	}
 
	// be sure that all threads are ended..
	for (int z=0; z < nThreads; z++){
		wThreads[z]->wait();
	}
 
	// ..continue with serial code...
 
	for (int i=0; i < nThreads; i++){
		delete wThreads[i]; // deleting the threads
	}
}
 
void
Manager_farmers_threads::assignJob(Agent_space* agent_h, const Pixel* plot_h){
	// .. this code shuld be run serialised by the main thread..
	agent = agent_h;
	plot = plot_h;
	agent->cachedOffer = 0;
}
 
void
Manager_farmers_threads::run(){
	// This code should be run parallelised...
	QMutex localMutex;
	localMutex.lock();
	agent->cachedOffer = agent->offerRentalPrice(plot);
	localMutex.unlock();
}

void
Manager_farmers::landAllocation(){
	// .. serial code ..

	for (int i=0; i < nThreads; i++){
		Manager_farmers_threads* wThread = new Manager_farmers_threads;
		wThreads.push_back(wThread);
	}

	// parallelise the code to z workingThreads.. 
	for (uint y=0;y<myAgents.size();y++){
		bool assigned = false;
		while(!assigned) {
			for (uint z=0;z<wThreads.size();z++){
			  if (!wThreads[z]->isRunning()){
				wThreads[z]->assignJob(myAgents[y], freePlot);
				wThreads[z]->start();
				assigned = true;
				break;
			  }
			}
		}	
	}

	// be sure that all threads are ended..
	for (int z=0; z < nThreads; z++){
		wThreads[z]->wait();
	}

	// ..continue with serial code...
	
	for (int i=0; i < nThreads; i++){
		delete wThreads[i]; // deleting the threads
	}
}

void
Manager_farmers_threads::assignJob(Agent_space* agent_h, const Pixel* plot_h){
	// .. this code shuld be run serialised by the main thread..
	agent = agent_h;
	plot = plot_h;
	agent->cachedOffer = 0;
}

void
Manager_farmers_threads::run(){
	// This code should be run parallelised...
	QMutex localMutex;
	localMutex.lock();
	agent->cachedOffer = agent->offerRentalPrice(plot);
	localMutex.unlock();
}

To copy to clipboard, switch view to plain text mode 

The question is.. why for hell it is crashing even if all the code is locked ?

[wysota]

You are creating a local mutex so each entry to the method creates a private instance of the mutex so each thread works on its own instance of the mutex so there is never a situation when two threads try to lock the same mutex. You have to work on a single mutex for all threads.

[sylvaticus]

Hi.. thanks for the answer.
In fact I originally tried to use the QMutex as a member of the thread class, like it is done in the book "C++ GUI programming with Qt4" ("Syncronizing threads").

The thread class looks now like this:

class Manager_farmers_threads : public QThread {
	Q_OBJECT
 
public:
	Manager_farmers_threads();
	void                assignJob(Agent_space* agent_h, const Pixel*plot_h);
 
protected:
	void run();
 
private:
	volatile Agent_space* agent;
	const Pixel* plot;
	QMutex mutex;
};

class Manager_farmers_threads : public QThread {
	Q_OBJECT

public:
	Manager_farmers_threads();
	void                assignJob(Agent_space* agent_h, const Pixel*plot_h);

protected:
	void run();

private:
	volatile Agent_space* agent;
	const Pixel* plot;
	QMutex mutex;
};

To copy to clipboard, switch view to plain text mode 

However it still doesn't work (and it should't, as each thread would still have it's own mutex istance).

If I understood correctly, you suggest to create the Mutex somewhere in the main thread and then giving pointers to the threads? Or create a function in the main thread to lock/unlock the mutex and call it from the threads?

[wysota]

This changes nothing. The mutex has to be static or global - different instances of the thread have to access the same instance of the mutex. The fact that the variable name of two mutexes is the same doesn't mean they are the same mutex.

[sylvaticus]

I created the Mutex in the main thread and gived threads a shared pointer, but still crashing (when I set nThreads=1 it's all going fine)

Now the code looks like that:

void
Manager_farmers::landAllocation(){
	// .. serial code ..
 
	QMutex* mutex = new QMutex;
	for (int i=0; i < nThreads; i++){
		Manager_farmers_threads* wThread = new Manager_farmers_threads;
		wThreads.push_back(wThread);
	}
 
	// parallelise the code to z workingThreads.. 
	for (uint y=0;y<myAgents.size();y++){
		bool assigned = false;
		while(!assigned) {
			for (uint z=0;z<wThreads.size();z++){
			  if (!wThreads[z]->isRunning()){
				wThreads[z]->assignJob(myAgents[y], freePlot, mutex);
				wThreads[z]->start();
				assigned = true;
				break;
			  }
			}
		}	
	}
 
	// be sure that all threads are ended..
	for (int z=0; z < nThreads; z++){
		wThreads[z]->wait();
	}
 
	// ..continue with serial code...
 
	for (int i=0; i < nThreads; i++){
		delete wThreads[i]; // deleting the threads
	}
	delete mutex;
}
 
void
Manager_farmers_threads::assignJob(Agent_space* agent_h, const Pixel* plot_h, QMutex* mutex_h){
	// .. this code shuld be run serialised by the main thread..
	agent = agent_h;
	plot = plot_h;
	mutex = mutex_h;
	agent->cachedOffer = 0;
}
 
void
Manager_farmers_threads::run(){
	// This code should be run parallelised...
	mutex.lock();
	agent->cachedOffer = agent->offerRentalPrice(plot);
	mutex.unlock();
}
 
 
class Manager_farmers_threads : public QThread {
    Q_OBJECT
 
public:
    Manager_farmers_threads();
    void                assignJob(Agent_space* agent_h, const Pixel*plot_h, QMutex* mutex_h);
 
protected:
    void run();
 
private:
    volatile Agent_space* agent;
    const Pixel* plot;
    QMutex* mutex;
};

void
Manager_farmers::landAllocation(){
	// .. serial code ..

	QMutex* mutex = new QMutex;
	for (int i=0; i < nThreads; i++){
		Manager_farmers_threads* wThread = new Manager_farmers_threads;
		wThreads.push_back(wThread);
	}

	// parallelise the code to z workingThreads.. 
	for (uint y=0;y<myAgents.size();y++){
		bool assigned = false;
		while(!assigned) {
			for (uint z=0;z<wThreads.size();z++){
			  if (!wThreads[z]->isRunning()){
				wThreads[z]->assignJob(myAgents[y], freePlot, mutex);
				wThreads[z]->start();
				assigned = true;
				break;
			  }
			}
		}	
	}

	// be sure that all threads are ended..
	for (int z=0; z < nThreads; z++){
		wThreads[z]->wait();
	}

	// ..continue with serial code...
	
	for (int i=0; i < nThreads; i++){
		delete wThreads[i]; // deleting the threads
	}
	delete mutex;
}

void
Manager_farmers_threads::assignJob(Agent_space* agent_h, const Pixel* plot_h, QMutex* mutex_h){
	// .. this code shuld be run serialised by the main thread..
	agent = agent_h;
	plot = plot_h;
	mutex = mutex_h;
	agent->cachedOffer = 0;
}

void
Manager_farmers_threads::run(){
	// This code should be run parallelised...
	mutex.lock();
	agent->cachedOffer = agent->offerRentalPrice(plot);
	mutex.unlock();
}


class Manager_farmers_threads : public QThread {
    Q_OBJECT
 
public:
    Manager_farmers_threads();
    void                assignJob(Agent_space* agent_h, const Pixel*plot_h, QMutex* mutex_h);
 
protected:
    void run();
 
private:
    volatile Agent_space* agent;
    const Pixel* plot;
    QMutex* mutex;
};

To copy to clipboard, switch view to plain text mode 

[wysota]

What is the Pixel class?

[sylvaticus]

Thank you for your answers.
"Pixel" it's a non-gui class that describes the space in my model.
Basically the parallelised code needs just read-only data on information stored on "pixels" and the idea is to eventually protect with mutex this kind of parts only, but the program crash even if I lock/unlock ALL the parallelised code (this is the funy side, becasue according on how I understood mutex looking there should not be any parallelised code when you lock it all ) .
Rendering of the maps on screen is done much later on in the serial part of the model.

Any how, this is the documentation for the Pixel class:
http://www.regmas.org/doc/referenceM...lassPixel.html

[wysota]

Where exactly does the application crash? What does the debugger say?

[sylvaticus]

Sorry for the late response (I moved to an area without internet :-( ).

The application crash when I use a call to a mathematical programming library named "glpk".

"Program received signal SIGABRT (Aborted)"

// solving the problem with the integer variables
stat=lpx_integer(lpglpk);

The re-entrant capability of this library is not assured, but the point is that it is wholly called from agent->cachedOffer = agent->offerRentalPrice(plot); that is protected by a mutex.
By the way all the steps of solving the mathematical problem (initialise the problem, solving, getting the solution and deleting the problem) are done within the "offerRentalPrice()" function.
So my question is: why even if I place a mutex this function is executed concurrently?

[wysota]

Please introduce debug statements around locking and unlocking the mutex so that you can check the sequence of the calls. You can use QThread::currentThreadId() to provide information which thread is performing the lock/unlock operation.

[sylvaticus]

I put debugging messages around the two lock/unlocks:

void
Manager_farmers_threads::run(){
	int threadId = currentThreadId();
	cout <<"A "<<threadId<<endl;
	mutex->lock();
	cout <<"B "<<threadId<<endl;
 
	( (Agent_farmer*) agent)->cachedOffer = ( (Agent_farmer*) agent)->offerRentalPrice(plot);
 
	cout <<"C "<<threadId<<endl;
	mutex->unlock();
	cout <<"D "<<threadId<<endl;
}

void
Manager_farmers_threads::run(){
	int threadId = currentThreadId();
	cout <<"A "<<threadId<<endl;
	mutex->lock();
	cout <<"B "<<threadId<<endl;
	
	( (Agent_farmer*) agent)->cachedOffer = ( (Agent_farmer*) agent)->offerRentalPrice(plot);
	
	cout <<"C "<<threadId<<endl;
	mutex->unlock();
	cout <<"D "<<threadId<<endl;
}

To copy to clipboard, switch view to plain text mode 

I'm attaching the output log...

This is an other log with only ABCD:

************************************************** *****************
*** !! Welcome to RegMAS - Regional Multi Agent Simulator !! ***
*** For info & doc: http://www.regmas.org/doc ***
*** Compiled on: Jul 3 2009 - 14:10:18 ***
************************************************** *****************

Selected scenario: default
**WARNING: No elements in the XML file. Expected at least one of type reclassRule
**WARNING: No elements in the XML file. Expected at least one of type reclassRule
ABCDABCDABCDABCDABCDABCDABACDBCDABCDABACDBCDABCDAB ACDBCDABCDABACDBCDAB

The external library crash only if I use nThreads > 1.
Furthermore, the second log were generated using the original, non-safe library, while the first log - crashing much later during the execution - were generated using a dev version of that library that is going in the direction of thread-safe.

So my question is: if I have a thread-unsafe library and I try to access it from different threads, even if I access it in a serialised way (using mutexes), it could be any how dangerous.
Is this sentence correct?