I fixed the deadlock in trunk (svn r7361).
Not sure whether it's the best way, but it works.
Here is the patch I used:
Index: parser.cpp
===================================================================
--- parser.cpp (Revision 7360)
+++ parser.cpp (Arbeitskopie)
@@ -1001,12 +1001,14 @@
bool Parser::ForceStartParsing()
{
- wxCriticalSectionLocker locker(s_ParserCritical);
if (!m_IsParsing && !m_BatchTimer.IsRunning() && !Done())
{
- m_BatchTimer.Start(100, wxTIMER_ONE_SHOT);
if (m_ParsingType == ptUndefined)
+ {
+ wxCriticalSectionLocker locker(s_ParserCritical);
m_ParsingType = ptAddFileToParser;
+ }
+ m_BatchTimer.Start(100, wxTIMER_ONE_SHOT);
return true;
}
else
The deadlock has occured, because ForceStartParsing uses s_ParserCrtical as wxCriticalSectionLocker and Done() does the same.
I moved the locker into the body of the if-clause, because that's the only place where something is changed.
I also moved the timer start, so it is surely fired after changing the parsing-type.
@cc-gurus (Loaden and ollydbg) :
please check the changes, I did not find any side-effects.
Many thanks!
But I have a question: m_IsParsing will access from a sub-thread.
If we only read a value, like m_IsParsing, m_TaskPool (stl::set), and do not write it, you can sure we not need add a locker for thread safe?
e.g. Done function:
bool Parser::Done()
{
wxCriticalSectionLocker locker(s_ParserCritical);
bool done = m_PriorityHeaders.empty()
&& m_SystemPriorityHeaders.empty()
&& m_BatchParseFiles.empty()
&& m_PredefinedMacros.IsEmpty()
&& !m_NeedMarkFileAsLocal
&& m_PoolTask.empty()
&& m_Pool.Done();
return done;
}
In Here the function only read the members, no write, so we must use a locker or noneed?
In Here the function only read the members, no write, so we must use a locker or noneed?
If another thread might change the content of any of the variables you are checking, a lock is needed. Basically you don't want parallel read and write access. If you can ensure all threads are only reading at the same time, the locker would not be needed.
Is it possible, that m_IsParsing is changed from false to true, while we are in ForceStartParsing ?
If yes and we need the locker, we could probably split the if-clause, like this:
if (!Done())
{
wxCriticalSectionLocker locker(s_ParserCritical);
if (!m_IsParsing && !m_BatchTimer.IsRunning())
{
if (m_ParsingType == ptUndefined)
m_ParsingType = ptAddFileToParser;
m_BatchTimer.Start(100, wxTIMER_ONE_SHOT);
return true;
}
}
That should also avoid the deadlock, but lock m_IsParsing and m_BatchTimer.IsRunning() (if the second is needed).