PDA

View Full Version : Prevent concurrent access from different threads?



agarny
25th February 2013, 19:13
Hi, I was wondering whether I could have some views on the following.

My application uses a thread to run a simulation and generate simulation data. A thread is used so that my application's GUI can remain responsive. Now, while a simulation is being run, my application's main thread makes successive calls to QTimer::singleShot() to fetch any new simulation data that has been generated, as well as to plot whatever the user wants to visualise. The user can decide on what to visualise by creating/removing traces, this through my application's GUI.

If I pre-select some traces and run a simulation, then everything works as expected. However, if, during a simulation, I remove a trace, then my application may crash. This is because the fetching function I call from QTimer::singleShot() goes through all the different traces and update them. Now, if a trace has been removed during a simulation and, more importantly, in the middle of my fetching function, then my fetching function might try to use a trace which doesn't exist anymore, hence the crash.

So, what would you do to avoid this problem? I originally thought I could solve this problem by using a mutex, but that failed and I believe this might be because of my fetching function being called through QTimer::singleShot() and therefore running in its own thread?

Anyway, any idea/help on the above would be much appreciated...

Cheers, Alan.

amleto
25th February 2013, 19:31
mutexes/critical sections stop concurrent access from different threads.

without seeing your code (read my sig), we can't say much more. It all depends how things go wrong when X uses Y that doesn't exist.

You should ask yourself: If Y doesn't exist, why & how can X use it?

Simply, you need a thread safe mechanism to remove traces - you will need to...

Analyse where it is safe to do this (the range of the critical section(s)).
Implement a mechanism (probably using signals/slots) that sends intention to remove traces from gui to simulation.

wysota
25th February 2013, 20:28
because of my fetching function being called through QTimer::singleShot() and therefore running in its own thread?

Timers do not start any new threads.

If using a mutex fails then apparently you're protecting the wrong resource or you're not protecting any resources at all.

agarny
26th February 2013, 10:41
amleto, I already use signals/slots to add/remove a trace (since it's done through my application's GUI). Anyway, you asked for some code, but it's not always easy to provide some but I have tried below...

wysota, thanks, I wasn't sure about QTimer::singleShot() creating a new thread or not. I should have checked Qt's code and well... now, I have...

My .cpp and .h files are as follows (to make it 'short', I removed comments and, I know, what I believe to be irrelevant code):


class SingleCellSimulationViewWidget : public Core::ViewWidget
{
Q_OBJECT

public:
...

private:
...

QMutex mSimulationMutex;
QMutex mTracesMutex;

void updateResults(SingleCellSimulationViewSimulation *pSimulation,
const qulonglong &pSize,
const bool &pReplot = false);

void checkResults(SingleCellSimulationViewSimulation *pSimulation);

...

private Q_SLOTS:
...

void resetFileTabIcon();

void showHideParameterPlot(const QString &pFileName,
CellMLSupport::CellmlFileRuntimeModelParameter *pParameter,
const bool &pShowParameterPlot);

void callCheckResults();

...
};


void SingleCellSimulationViewWidget::resetFileTabIcon()
{
SingleCellSimulationViewSimulation *simulation;

{
QMutexLocker mutexLocker(&mSimulationMutex);

simulation = mStoppedSimulations.first();

mStoppedSimulations.removeFirst();
}

emit updateFileTabIcon(simulation->fileName(), QIcon());
}

void SingleCellSimulationViewWidget::showHideParameterP lot(const QString &pFileName,
CellMLSupport::CellmlFileRuntimeModelParameter *pParameter,
const bool &pShowParameterPlot)
{
QString key = parameterKey(pFileName, pParameter);

QMutexLocker mutexLocker(&mTracesMutex);
QwtPlotCurve *trace = mTraces.value(key);

if (trace && !pShowParameterPlot) {
mActiveGraphPanel->plot()->removeTrace(trace);

mTraces.remove(key);
} else if (!trace && pShowParameterPlot) {
SingleCellSimulationViewSimulationResults *results = mSimulation->results();

double *yData;

...

QwtPlotCurve *trace = mActiveGraphPanel->plot()->addTrace(results->points(), yData, results->size());

mTraces.insert(key, trace);
}
}

void SingleCellSimulationViewWidget::updateResults(Sing leCellSimulationViewSimulation *pSimulation,
const qulonglong &pSize,
const bool &pReplot)
{
SingleCellSimulationViewSimulation *simulation = pSimulation?pSimulation:mSimulation;

if (simulation == mSimulation) {
QMutexLocker mutexLocker(&mTracesMutex);

QMap<QString, QwtPlotCurve *>::const_iterator iter = mTraces.constBegin();

while (iter != mTraces.constEnd()) {
QString fileName = iter.key();

...

QwtPlotCurve *trace = iter.value();

if (!fileName.compare(mSimulation->fileName())) {
double *yData;

...

qulonglong oldSize = trace->dataSize();

trace->setRawSamples(simulation->results()->points(), yData, pSize);

if (!pReplot && (pSize > 1))
qobject_cast<SingleCellSimulationViewGraphPanelPlotWidget *>(trace->plot())->drawTraceSegment(trace, oldSize?oldSize-1:0, pSize-1);
}

++iter;
}

...
} else {
...
}
}

void SingleCellSimulationViewWidget::checkResults(Singl eCellSimulationViewSimulation *pSimulation)
{
qulonglong simulationResultsSize = pSimulation->results()->size();

if (simulationResultsSize != mOldSimulationResultsSizes.value(pSimulation)) {
mOldSimulationResultsSizes.insert(pSimulation, simulationResultsSize);

updateResults(pSimulation, simulationResultsSize);
}

if (pSimulation->isRunning()) {
mCheckResultsSimulations << pSimulation;

QTimer::singleShot(0, this, SLOT(callCheckResults()));
}
}

void SingleCellSimulationViewWidget::callCheckResults()
{
SingleCellSimulationViewSimulation *simulation;

{
QMutexLocker mutexLocker(&mSimulationMutex);

simulation = mCheckResultsSimulations.first();

mCheckResultsSimulations.removeFirst();
}

checkResults(simulation);
}

wysota
26th February 2013, 11:51
And where are the threads involved here? I see a couple of problems with your code related to threading but first I'd like to know if we do have any multithreading here at all.

agarny
26th February 2013, 12:44
And where are the threads involved here? I see a couple of problems with your code related to threading but first I'd like to know if we do have any multithreading here at all.

Exactly, the mutexes I use in the code I gave are all in functions called from the same thread and therefore unnecessary. I have just had a chat with someone and to cut a long story short, I needed to learn more about Qt's event loop, and now that I know a bit more about it, I can see that my mutexes were unnecessary and the fact that my application crashes is most likely due to some non-reentrant / non-atomic functions, from two different threads, which access some shared data for reading and/or writing purposes. So, yes, I am still very much learning (the hard way) about threading...