Permalink
Browse files

Bug 756241 - Race condition in parallel DOT runs

  • Loading branch information...
Dimitri van Heesch
Dimitri van Heesch committed Oct 17, 2015
1 parent ffff695 commit f196c9f1d69238a814ff3152103f3bd310efdf0d
Showing with 77 additions and 55 deletions.
  1. +2 −2 qtools/Doxyfile
  2. +38 −44 src/dot.cpp
  3. +37 −9 src/dot.h
@@ -279,7 +279,7 @@ PERL_PATH = /usr/bin/perl
CLASS_DIAGRAMS = YES
MSCGEN_PATH =
HIDE_UNDOC_RELATIONS = YES
HAVE_DOT = NO
HAVE_DOT = YES
DOT_NUM_THREADS = 0
DOT_FONTNAME =
DOT_FONTSIZE = 10
@@ -306,4 +306,4 @@ MAX_DOT_GRAPH_DEPTH = 0
DOT_TRANSPARENT = YES
DOT_MULTI_TARGETS = NO
GENERATE_LEGEND = YES
DOT_CLEANUP = NO
DOT_CLEANUP = YES
@@ -659,10 +659,9 @@ static bool writeSVGFigureLink(FTextStream &out,const QCString &relPath,
// since dot silently reproduces the input file when it does not
// support the PNG format, we need to check the result.
static void checkDotResult(const QCString &imgName)
static void checkDotResult(const char *imgExt, const char *imgName)
{
QCString imgExt = getDotImageExtension();
if (imgExt=="png")
if (qstrcmp(imgExt,"png")==0)
{
FILE *f = portable_fopen(imgName,"rb");
if (f)
@@ -675,19 +674,19 @@ static void checkDotResult(const QCString &imgName)
err("Image `%s' produced by dot is not a valid PNG!\n"
"You should either select a different format "
"(DOT_IMAGE_FORMAT in the config file) or install a more "
"recent version of graphviz (1.7+)\n",imgName.data()
"recent version of graphviz (1.7+)\n",imgName
);
}
}
else
{
err("Could not read image `%s' generated by dot!\n",imgName.data());
err("Could not read image `%s' generated by dot!\n",imgName);
}
fclose(f);
}
else
{
err("Could not open image `%s' generated by dot!\n",imgName.data());
err("Could not open image `%s' generated by dot!\n",imgName);
}
}
}
@@ -793,54 +792,46 @@ int DotNodeList::compareValues(const DotNode *n1,const DotNode *n2) const
DotRunner::DotRunner(const QCString &file,const QCString &path,
bool checkResult,const QCString &imageName)
: m_file(file), m_path(path),
m_checkResult(checkResult), m_imageName(imageName)
{
static bool dotCleanUp = Config_getBool("DOT_CLEANUP");
m_cleanUp = dotCleanUp;
: m_dotExe(Config_getString("DOT_PATH")+"dot"),
m_file(file), m_path(path),
m_checkResult(checkResult), m_imageName(imageName),
m_imgExt(getDotImageExtension())
{
static bool dotCleanUp = Config_getBool("DOT_CLEANUP");
static bool dotMultiTargets = Config_getBool("DOT_MULTI_TARGETS");
m_cleanUp = dotCleanUp;
m_multiTargets = dotMultiTargets;
m_jobs.setAutoDelete(TRUE);
}
void DotRunner::addJob(const char *format,const char *output)
{
QCString args = QCString("-T")+format+" -o \""+output+"\"";
m_jobs.append(new QCString(args));
m_jobs.append(new DotConstString(args));
}
void DotRunner::addPostProcessing(const char *cmd,const char *args)
{
m_postCmd = cmd;
m_postArgs = args;
m_postCmd.set(cmd);
m_postArgs.set(args);
}
bool DotRunner::run()
{
int exitCode=0;
// we need to use data here to make a copy of the string, as Config_getString can be called by
// multiple threads simulaneously and the reference counting is not thread safe.
QCString dotExe = Config_getString("DOT_PATH").data();
dotExe+="dot";
bool multiTargets = Config_getBool("DOT_MULTI_TARGETS");
QCString dotArgs;
QListIterator<QCString> li(m_jobs);
QCString *s;
QCString file = m_file;
QCString path = m_path;
QCString imageName = m_imageName;
QCString postCmd = m_postCmd;
QCString postArgs = m_postArgs;
bool checkResult = m_checkResult;
bool cleanUp = m_cleanUp;
if (multiTargets)
{
dotArgs="\""+file+"\"";
QListIterator<DotConstString> li(m_jobs);
DotConstString *s;
if (m_multiTargets)
{
dotArgs=QCString("\"")+m_file.data()+"\"";
for (li.toFirst();(s=li.current());++li)
{
dotArgs+=' ';
dotArgs+=*s;
dotArgs+=s->data();
}
if ((exitCode=portable_system(dotExe,dotArgs,FALSE))!=0)
if ((exitCode=portable_system(m_dotExe.data(),dotArgs,FALSE))!=0)
{
goto error;
}
@@ -849,30 +840,33 @@ bool DotRunner::run()
{
for (li.toFirst();(s=li.current());++li)
{
dotArgs="\""+file+"\" "+*s;
if ((exitCode=portable_system(dotExe,dotArgs,FALSE))!=0)
dotArgs=QCString("\"")+m_file.data()+"\" "+s->data();
if ((exitCode=portable_system(m_dotExe.data(),dotArgs,FALSE))!=0)
{
goto error;
}
}
}
if (!postCmd.isEmpty() && portable_system(postCmd,postArgs)!=0)
if (!m_postCmd.isEmpty() && portable_system(m_postCmd.data(),m_postArgs.data())!=0)
{
err("Problems running '%s' as a post-processing step for dot output\n",m_postCmd.data());
return FALSE;
}
if (checkResult) checkDotResult(imageName);
if (cleanUp)
if (m_checkResult)
{
checkDotResult(m_imgExt.data(),m_imageName.data());
}
if (m_cleanUp)
{
//printf("removing dot file %s\n",m_file.data());
//QDir(path).remove(file);
m_cleanupItem.file = file;
m_cleanupItem.path = path;
m_cleanupItem.file.set(m_file.data());
m_cleanupItem.path.set(m_path.data());
}
return TRUE;
error:
err("Problems running dot: exit code=%d, command='%s', arguments='%s'\n",
exitCode,dotExe.data(),dotArgs.data());
exitCode,m_dotExe.data(),dotArgs.data());
return FALSE;
}
@@ -1202,7 +1196,7 @@ void DotWorkerThread::run()
while ((runner=m_queue->dequeue()))
{
runner->run();
DotRunner::CleanupItem cleanup = runner->cleanup();
const DotRunner::CleanupItem &cleanup = runner->cleanup();
if (!cleanup.file.isEmpty())
{
m_cleanupItems.append(new DotRunner::CleanupItem(cleanup));
@@ -1216,7 +1210,7 @@ void DotWorkerThread::cleanup()
DotRunner::CleanupItem *ci;
for (;(ci=it.current());++it)
{
QDir(ci->path).remove(ci->file);
QDir(ci->path.data()).remove(ci->file.data());
}
}
@@ -4237,7 +4231,7 @@ void writeDotGraphFromFile(const char *inFile,const char *outDir,
return;
}
if (format==GOF_BITMAP) checkDotResult(absImgName);
if (format==GOF_BITMAP) checkDotResult(getDotImageExtension(),absImgName);
Doxygen::indexList->addImageFile(imgName);
@@ -326,15 +326,40 @@ class DotGroupCollaboration
QList<Edge> m_edges;
};
/** Minimal constant string class that is thread safe, once initialized. */
class DotConstString
{
public:
DotConstString() { m_str=0; }
~DotConstString() { delete[] m_str; }
DotConstString(const QCString &s) : m_str(0) { set(s); }
DotConstString(const DotConstString &s) : m_str(0) { set(s.data()); }
const char *data() const { return m_str; }
bool isEmpty() const { return m_str==0 || m_str[0]=='\0'; }
void set(const QCString &s)
{
delete[] m_str;
m_str=0;
if (!s.isEmpty())
{
m_str=new char[s.length()+1];
qstrcpy(m_str,s.data());
}
}
private:
DotConstString &operator=(const DotConstString &);
char *m_str;
};
/** Helper class to run dot from doxygen.
*/
class DotRunner
{
public:
struct CleanupItem
{
QCString path;
QCString file;
DotConstString path;
DotConstString file;
};
/** Creates a runner for a dot \a file. */
@@ -352,16 +377,19 @@ class DotRunner
/** Runs dot for all jobs added. */
bool run();
CleanupItem cleanup() const { return m_cleanupItem; }
const CleanupItem &cleanup() const { return m_cleanupItem; }
private:
QList<QCString> m_jobs;
QCString m_postArgs;
QCString m_postCmd;
QCString m_file;
QCString m_path;
DotConstString m_dotExe;
bool m_multiTargets;
QList<DotConstString> m_jobs;
DotConstString m_postArgs;
DotConstString m_postCmd;
DotConstString m_file;
DotConstString m_path;
bool m_checkResult;
QCString m_imageName;
DotConstString m_imageName;
DotConstString m_imgExt;
bool m_cleanUp;
CleanupItem m_cleanupItem;
};

0 comments on commit f196c9f

Please sign in to comment.