walkero
/
cppcheck
1
0
Fork 0
Code Issues Pull Requests Projects Releases 1 Wiki Activity

Refactor ThreadExecutor::check() to handle child failures more gracefully

This commit is contained in:
Greg Hewgill 2011-03-05 16:43:22 +13:00
parent 957bb5c0f2
commit 5bbf39d094
3 changed files with 93 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Makefile
View File

@ -330,7 +330,7 @@ test/teststl.o: test/teststl.cpp lib/tokenize.h lib/checkstl.h lib/check.h lib/t
test/testsuite.o: test/testsuite.cpp test/testsuite.h lib/errorlogger.h lib/settings.h test/redirect.h test/options.h test/testsuite.o: test/testsuite.cpp test/testsuite.h lib/errorlogger.h lib/settings.h test/redirect.h test/options.h
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuite.o test/testsuite.cpp $(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuite.o test/testsuite.cpp
test/testsuppressions.o: test/testsuppressions.cpp lib/cppcheck.h lib/settings.h lib/errorlogger.h lib/checkunusedfunctions.h lib/check.h lib/token.h lib/tokenize.h test/testsuite.h test/redirect.h test/testsuppressions.o: test/testsuppressions.cpp lib/cppcheck.h lib/settings.h lib/errorlogger.h lib/checkunusedfunctions.h lib/check.h cli/threadexecutor.h lib/token.h lib/tokenize.h test/testsuite.h test/redirect.h
$(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuppressions.o test/testsuppressions.cpp $(CXX) $(CPPFLAGS) $(CXXFLAGS) ${INCLUDE_FOR_TEST} -c -o test/testsuppressions.o test/testsuppressions.cpp
test/testsymboldatabase.o: test/testsymboldatabase.cpp test/testsuite.h lib/errorlogger.h lib/settings.h test/redirect.h test/testutils.h lib/tokenize.h lib/token.h lib/symboldatabase.h test/testsymboldatabase.o: test/testsymboldatabase.cpp test/testsuite.h lib/errorlogger.h lib/settings.h test/redirect.h test/testutils.h lib/tokenize.h lib/token.h lib/symboldatabase.h

105
cli/threadexecutor.cpp
View File

@ -35,7 +35,7 @@ ThreadExecutor::ThreadExecutor(const std::vector<std::string> &filenames, Settin
: _filenames(filenames), _settings(settings), _errorLogger(errorLogger), _fileCount(0) : _filenames(filenames), _settings(settings), _errorLogger(errorLogger), _fileCount(0)
{ {
#ifdef THREADING_MODEL_FORK #ifdef THREADING_MODEL_FORK
_pipe[0] = _pipe[1] = 0; _wpipe = 0;
#endif #endif
} }
@ -55,10 +55,10 @@ void ThreadExecutor::addFileContent(const std::string &path, const std::string &
#ifdef THREADING_MODEL_FORK #ifdef THREADING_MODEL_FORK
int ThreadExecutor::handleRead(unsigned int &result) int ThreadExecutor::handleRead(int rpipe, unsigned int &result)
{ {
char type = 0; char type = 0;
if (read(_pipe[0], &type, 1) <= 0) if (read(rpipe, &type, 1) <= 0)
{ {
if (errno == EAGAIN) if (errno == EAGAIN)
return 0; return 0;
@ -73,14 +73,14 @@ int ThreadExecutor::handleRead(unsigned int &result)
} }
unsigned int len = 0; unsigned int len = 0;
if (read(_pipe[0], &len, sizeof(len)) <= 0) if (read(rpipe, &len, sizeof(len)) <= 0)
{ {
std::cerr << "#### You found a bug from cppcheck.\nThreadExecutor::handleRead error, type was:" << type << std::endl; std::cerr << "#### You found a bug from cppcheck.\nThreadExecutor::handleRead error, type was:" << type << std::endl;
exit(0); exit(0);
} }
char *buf = new char[len]; char *buf = new char[len];
if (read(_pipe[0], buf, len) <= 0) if (read(rpipe, buf, len) <= 0)
{ {
std::cerr << "#### You found a bug from cppcheck.\nThreadExecutor::handleRead error, type was:" << type << std::endl; std::cerr << "#### You found a bug from cppcheck.\nThreadExecutor::handleRead error, type was:" << type << std::endl;
exit(0); exit(0);
@ -134,32 +134,35 @@ unsigned int ThreadExecutor::check()
{ {
_fileCount = 0; _fileCount = 0;
unsigned int result = 0; unsigned int result = 0;
if (pipe(_pipe) == -1)
std::list<int> rpipes;
std::map<pid_t, std::string> childFile;
unsigned int i = 0;
while (true)
{
// Start a new child
if (i < _filenames.size() && rpipes.size() < _settings._jobs)
{
int pipes[2];
if (pipe(pipes) == -1)
{ {
perror("pipe"); perror("pipe");
exit(1); exit(1);
} }
int flags = 0; int flags = 0;
if ((flags = fcntl(_pipe[0], F_GETFL, 0)) < 0) if ((flags = fcntl(pipes[0], F_GETFL, 0)) < 0)
{ {
perror("fcntl"); perror("fcntl");
exit(1); exit(1);
} }
if (fcntl(_pipe[0], F_SETFL, flags | O_NONBLOCK) < 0) if (fcntl(pipes[0], F_SETFL, flags | O_NONBLOCK) < 0)
{ {
perror("fcntl"); perror("fcntl");
exit(1); exit(1);
} }
unsigned int childCount = 0;
unsigned int i = 0;
while (true)
{
// Start a new child
if (i < _filenames.size() && childCount < _settings._jobs)
{
pid_t pid = fork(); pid_t pid = fork();
if (pid < 0) if (pid < 0)
{ {
@ -169,6 +172,9 @@ unsigned int ThreadExecutor::check()
} }
else if (pid == 0) else if (pid == 0)
{ {
close(pipes[0]);
_wpipe = pipes[1];
CppCheck fileChecker(*this, false); CppCheck fileChecker(*this, false);
fileChecker.settings(_settings); fileChecker.settings(_settings);
@ -190,31 +196,70 @@ unsigned int ThreadExecutor::check()
exit(0); exit(0);
} }
++childCount; close(pipes[1]);
rpipes.push_back(pipes[0]);
childFile[pid] = _filenames[i];
++i; ++i;
} }
else if (childCount > 0) else if (!rpipes.empty())
{ {
// Wait for child to quit before stating new processes fd_set rfds;
while (true) FD_ZERO(&rfds);
for (std::list<int>::const_iterator rp = rpipes.begin(); rp != rpipes.end(); ++rp)
FD_SET(*rp, &rfds);
int r = select(*std::max_element(rpipes.begin(), rpipes.end()) + 1, &rfds, NULL, NULL, NULL);
if (r > 0)
{ {
int readRes = handleRead(result); std::list<int>::iterator rp = rpipes.begin();
while (rp != rpipes.end())
{
if (FD_ISSET(*rp, &rfds))
{
int readRes = handleRead(*rp, result);
if (readRes == -1) if (readRes == -1)
break;
else if (readRes == 0)
{ {
struct timespec duration; close(*rp);
duration.tv_sec = 0; rp = rpipes.erase(rp);
duration.tv_nsec = 5 * 1000 * 1000; // 5 ms }
nanosleep(&duration, NULL); else
++rp;
}
else
++rp;
} }
} }
int stat = 0; int stat = 0;
waitpid(0, &stat, 0); pid_t child = waitpid(0, &stat, WNOHANG);
--childCount; if (child > 0)
{
std::string childname;
std::map<pid_t, std::string>::iterator c = childFile.find(child);
if (c != childFile.end())
{
childname = c->second;
childFile.erase(c);
} }
else if (childCount == 0)
if (WIFSIGNALED(stat))
{
std::ostringstream oss;
oss << "Internal error: Child process crashed with signal " << WTERMSIG(stat);
std::list<ErrorLogger::ErrorMessage::FileLocation> locations;
locations.push_back(ErrorLogger::ErrorMessage::FileLocation(childname, 0));
const ErrorLogger::ErrorMessage errmsg(locations,
Severity::error,
oss.str(),
"cppcheckError");
_errorLogger.reportErr(errmsg);
}
}
}
else
{ {
// All done // All done
break; break;
@ -232,7 +277,7 @@ void ThreadExecutor::writeToPipe(char type, const std::string &data)
out[0] = type; out[0] = type;
std::memcpy(&(out[1]), &len, sizeof(len)); std::memcpy(&(out[1]), &len, sizeof(len));
std::memcpy(&(out[1+sizeof(len)]), data.c_str(), len); std::memcpy(&(out[1+sizeof(len)]), data.c_str(), len);
if (write(_pipe[1], out, len + 1 + sizeof(len)) <= 0) if (write(_wpipe, out, len + 1 + sizeof(len)) <= 0)
{ {
delete [] out; delete [] out;
out = 0; out = 0;

8
cli/threadexecutor.h
View File

@ -71,9 +71,13 @@ private:
* 0 if there is nothing in the pipe to be read * 0 if there is nothing in the pipe to be read
* 1 if we did read something * 1 if we did read something
*/ */
int handleRead(unsigned int &result); int handleRead(int rpipe, unsigned int &result);
void writeToPipe(char type, const std::string &data); void writeToPipe(char type, const std::string &data);
int _pipe[2]; /**
* Write end of status pipe, different for each child.
* Not used in master process.
*/
int _wpipe;
std::list<std::string> _errorList; std::list<std::string> _errorList;
public: public:
/** /**