From 3aef45d35bab536aaf3ad1ea040b70ea3f0612c0 Mon Sep 17 00:00:00 2001 From: marha Date: Wed, 6 Oct 2010 16:03:47 +0000 Subject: Solved reentrancy problem in commandqueue --- tools/mhmake/src/build.cpp | 4 +- tools/mhmake/src/commandqueue.cpp | 71 +++++++++++++++++++++++++---------- tools/mhmake/src/commandqueue.h | 26 ++++++------- tools/mhmake/src/fileinfo.cpp | 2 - tools/mhmake/src/fileinfo.h | 2 - tools/mhmake/src/mhmakefileparser.cpp | 8 ++-- tools/mhmake/src/mhmakefileparser.h | 4 +- tools/mhmake/src/util.h | 2 +- 8 files changed, 72 insertions(+), 47 deletions(-) diff --git a/tools/mhmake/src/build.cpp b/tools/mhmake/src/build.cpp index 1d4a6ddc1..bfd5d0211 100644 --- a/tools/mhmake/src/build.cpp +++ b/tools/mhmake/src/build.cpp @@ -240,7 +240,7 @@ void mhmakefileparser::CreatePythonExe(const string &FullCommand) #endif /*****************************************************************************/ -int mhmakefileparser::SearchPath(const char *szCommand, const char *pExt, int Len, char *szFullCommand,char **pFilePart) const +int mhmakefileparser::SearchPath(const char *szCommand, const char *pExt, size_t Len, char *szFullCommand,char **pFilePart) const { static vector< refptr > vSearchPath; @@ -284,7 +284,7 @@ int mhmakefileparser::SearchPath(const char *szCommand, const char *pExt, int Le found: string FullCommand=CommandFile->GetFullFileName(); - int CommandLen=FullCommand.size(); + size_t CommandLen=FullCommand.size(); if (CommandLen>Len-1) { throw string("Command to long: ") + FullCommand; diff --git a/tools/mhmake/src/commandqueue.cpp b/tools/mhmake/src/commandqueue.cpp index ba1a7d073..c09f505d7 100644 --- a/tools/mhmake/src/commandqueue.cpp +++ b/tools/mhmake/src/commandqueue.cpp @@ -60,6 +60,8 @@ commandqueue::commandqueue() : SYSTEM_INFO SysInfo; GetSystemInfo(&SysInfo); m_MaxNrCommandsInParallel=SysInfo.dwNumberOfProcessors; + + m_DummyWaitHandle=(mh_pid_t)CreateEvent(NULL,TRUE,FALSE,NULL); #else FILE *pFile=fopen("/proc/cpuinfo","r"); const char *pProc="\nprocessor"; @@ -81,16 +83,21 @@ commandqueue::commandqueue() : cur=0; } m_MaxNrCommandsInParallel=NrProcs; + m_DummyWaitHandle=(mh_pid_t)-1; + #endif m_pActiveProcesses=new mh_pid_t[m_MaxNrCommandsInParallel]; - m_pActiveEntries= new activeentry[m_MaxNrCommandsInParallel]; + m_pActiveEntries= new refptr[m_MaxNrCommandsInParallel]; } commandqueue::~commandqueue() { delete [] m_pActiveProcesses; delete [] m_pActiveEntries; + #ifdef WIN32 + CloseHandle(m_DummyWaitHandle); + #endif } void commandqueue::SetNrParallelBuilds(unsigned NrParallelBuilds) @@ -102,10 +109,10 @@ void commandqueue::SetNrParallelBuilds(unsigned NrParallelBuilds) delete [] m_pActiveProcesses; delete [] m_pActiveEntries; m_pActiveProcesses=new mh_pid_t[NrParallelBuilds]; - m_pActiveEntries= new activeentry[NrParallelBuilds]; + m_pActiveEntries= new refptr[NrParallelBuilds]; } -void commandqueue::ThrowCommandExecutionError(activeentry *pActiveEntry) +void commandqueue::ThrowCommandExecutionError(refptr pActiveEntry) { refptr pTarget=pActiveEntry->pTarget; const string &Command=pActiveEntry->Command; @@ -118,12 +125,22 @@ void commandqueue::ThrowCommandExecutionError(activeentry *pActiveEntry) throw ErrorMessage; } -void commandqueue::AddActiveEntry(activeentry &ActiveEntry, mh_pid_t ActiveProcess) +refptr commandqueue::CreateActiveEntry(void) { -//cout << "Adding entry "<GetQuotedFullFileName()<<" ("< pRet=new activeentry; + m_pActiveEntries[m_NrActiveEntries]=pRet; + m_pActiveProcesses[m_NrActiveEntries]=this->m_DummyWaitHandle; m_NrActiveEntries++; + return pRet; +} + +unsigned commandqueue::GetActiveEntryId(const refptr pActiveEntry) const +{ + unsigned i=0; + for (i=0; ipTarget->GetFullFileName()); } void commandqueue::RemoveActiveEntry(unsigned Entry) @@ -140,14 +157,14 @@ void commandqueue::RemoveActiveEntry(unsigned Entry) Entry=EntryP1; } } - m_pActiveEntries[Entry].clear(); + m_pActiveEntries[Entry]=NULL; m_pActiveProcesses[Entry]=NULL; m_NrActiveEntries--; } /* Start to execute next command, return true when command is completely executed upon return */ -bool commandqueue::StartExecuteNextCommand(activeentry *pActiveEntry, mh_pid_t *pActiveProcess) +bool commandqueue::StartExecuteNextCommand(refptr pActiveEntry, mh_pid_t *pActiveProcess) { refptr pTarget=pActiveEntry->pTarget; mhmakeparser *pMakefile=pTarget->GetRule()->GetMakefile(); @@ -189,7 +206,7 @@ bool commandqueue::StartExecuteNextCommand(activeentry *pActiveEntry, mh_pid_t * return true; } -void commandqueue::TargetBuildFinished(activeentry *pActiveEntry) +void commandqueue::TargetBuildFinished(refptr pActiveEntry) { refptr pTarget=pActiveEntry->pTarget; @@ -237,20 +254,20 @@ bool commandqueue::StartExecuteCommands(const refptr &pTarget) mhmakeparser *pMakefile=pRule->GetMakefile(); vector::iterator CommandIt=pRule->GetCommands().begin(); - activeentry ActiveEntry; + refptr pActiveEntry=CreateActiveEntry(); mh_pid_t ActiveProcess; - md5_starts( &ActiveEntry.md5ctx ); + md5_starts( &pActiveEntry->md5ctx ); - ActiveEntry.pTarget=pTarget; - ActiveEntry.CurrentCommandIt=CommandIt; + pActiveEntry->pTarget=pTarget; + pActiveEntry->CurrentCommandIt=CommandIt; while (1) { - if (StartExecuteNextCommand(&ActiveEntry, &ActiveProcess)) + if (StartExecuteNextCommand(pActiveEntry, &ActiveProcess)) { - ActiveEntry.CurrentCommandIt++; - if (ActiveEntry.CurrentCommandIt==pRule->GetCommands().end()) + pActiveEntry->CurrentCommandIt++; + if (pActiveEntry->CurrentCommandIt==pRule->GetCommands().end()) { // All commands executed break; @@ -258,11 +275,12 @@ bool commandqueue::StartExecuteCommands(const refptr &pTarget) } else { - AddActiveEntry(ActiveEntry,ActiveProcess); + m_pActiveProcesses[GetActiveEntryId(pActiveEntry)]=ActiveProcess; // We use GetActiveEntryId to avoid reentrancy problems return false; } } - TargetBuildFinished(&ActiveEntry); + TargetBuildFinished(pActiveEntry); + RemoveActiveEntry(pActiveEntry); return true; } @@ -296,10 +314,23 @@ mh_time_t commandqueue::WaitForTarget(const refptr &pTarget) while (1) { // First wait until one of the processes that are running is finished + if (m_NrActiveEntries==1 && m_pActiveProcesses[0]==this->m_DummyWaitHandle) + { + #ifdef _DEBUG + if (pTarget!=m_pActiveEntries[0]->pTarget) + throw("Wrong assumption: waiting for target " + pTarget->GetFullFileName() + " but in wait list is " + m_pActiveEntries[0]->pTarget->GetFullFileName()); + #endif + pTarget->SetDateToNow(); + return pTarget->GetDate(); // This is a reentrancy problem, assume that the target is just build + } unsigned Ret=WaitForMultipleObjects(m_NrActiveEntries,m_pActiveProcesses,FALSE,INFINITE); if (Ret>=m_NrActiveEntries) + #ifdef WIN32 + throw("fatal error: unexpected return value of WaitForMultipleObjects " + stringify(Ret) + " " + stringify(GetLastError())); + #else throw("fatal error: unexpected return value of WaitForMultipleObjects " + stringify(Ret)); - activeentry *pActiveEntry=&m_pActiveEntries[Ret]; + #endif + refptr pActiveEntry=m_pActiveEntries[Ret]; refptr pCurrentTarget=pActiveEntry->pTarget; refptr pRule=pCurrentTarget->GetRule(); diff --git a/tools/mhmake/src/commandqueue.h b/tools/mhmake/src/commandqueue.h index 3642d2f1d..d29057443 100644 --- a/tools/mhmake/src/commandqueue.h +++ b/tools/mhmake/src/commandqueue.h @@ -31,36 +31,34 @@ typedef pid_t mh_pid_t; class commandqueue { - struct activeentry + struct activeentry : public refbase { refptr pTarget; vector::const_iterator CurrentCommandIt; string Command; md5_context md5ctx; bool IgnoreError; - void clear() - { - pTarget=NULL; - Command.clear(); - #ifdef _DEBUG - md5ctx.Data.clear(); - #endif - } }; private: queue< refptr > m_Queue; unsigned m_MaxNrCommandsInParallel; mh_pid_t *m_pActiveProcesses; - activeentry *m_pActiveEntries; + refptr *m_pActiveEntries; unsigned m_NrActiveEntries; + mh_pid_t m_DummyWaitHandle; private: - void ThrowCommandExecutionError(activeentry *pActiveEntry); - void AddActiveEntry(activeentry &ActiveEntry, mh_pid_t ActiveProcess); + void ThrowCommandExecutionError(refptr pActiveEntry); + refptr CreateActiveEntry(void); + unsigned GetActiveEntryId(const refptr pActiveEntry) const; void RemoveActiveEntry(unsigned Entry); + void RemoveActiveEntry(refptr pActiveEntry) + { + RemoveActiveEntry(GetActiveEntryId(pActiveEntry)); + } bool StartExecuteCommands(const refptr &pTarget); - bool StartExecuteNextCommand(activeentry *pActiveEntry, mh_pid_t *pActiveProcess); - void TargetBuildFinished(activeentry *pActiveEntry); + bool StartExecuteNextCommand(refptr pActiveEntry, mh_pid_t *pActiveProcess); + void TargetBuildFinished(refptr pActiveEntry); public: commandqueue(); diff --git a/tools/mhmake/src/fileinfo.cpp b/tools/mhmake/src/fileinfo.cpp index fe9f18bce..d63724882 100644 --- a/tools/mhmake/src/fileinfo.cpp +++ b/tools/mhmake/src/fileinfo.cpp @@ -135,12 +135,10 @@ bool fileinfo::IsDir() const } /////////////////////////////////////////////////////////////////////////////// -#ifdef _DEBUG void fileinfo::SetDateToNow() { m_Date=time(NULL); } -#endif /////////////////////////////////////////////////////////////////////////////// string fileinfo::GetPrerequisits() const diff --git a/tools/mhmake/src/fileinfo.h b/tools/mhmake/src/fileinfo.h index e221293e6..3c7917be3 100644 --- a/tools/mhmake/src/fileinfo.h +++ b/tools/mhmake/src/fileinfo.h @@ -316,9 +316,7 @@ public: return m_IsPhony; } mh_time_t realGetDate(void); -#ifdef _DEBUG void SetDateToNow(void); -#endif void SetDate(mh_time_t Date) { diff --git a/tools/mhmake/src/mhmakefileparser.cpp b/tools/mhmake/src/mhmakefileparser.cpp index 725312b61..5236d58ee 100644 --- a/tools/mhmake/src/mhmakefileparser.cpp +++ b/tools/mhmake/src/mhmakefileparser.cpp @@ -928,7 +928,7 @@ void mhmakefileparser::SetExport(const string &Var, const string &Val) { while (*pEnd++); } - int Len=pEnd-pEnv+1; + size_t Len=pEnd-pEnv+1; m_pEnv=(char*)malloc(Len); memcpy(m_pEnv,pEnv,Len); m_EnvLen=Len; @@ -964,9 +964,9 @@ void mhmakefileparser::SetExport(const string &Var, const string &Val) } while (*pEnv++); } - int VarLen=Var.length(); - int ValLen=Val.length(); - int Extra=VarLen+ValLen+2; + size_t VarLen=Var.length(); + size_t ValLen=Val.length(); + size_t Extra=VarLen+ValLen+2; /* Add the variable at the end */ m_pEnv=(char*)realloc(m_pEnv,m_EnvLen+Extra); pEnv=m_pEnv+m_EnvLen-1; diff --git a/tools/mhmake/src/mhmakefileparser.h b/tools/mhmake/src/mhmakefileparser.h index 9c75c8771..19bb403e3 100644 --- a/tools/mhmake/src/mhmakefileparser.h +++ b/tools/mhmake/src/mhmakefileparser.h @@ -100,7 +100,7 @@ protected: #else char **m_pEnv; // New environment in case the makefile exports variables #endif - int m_EnvLen; // Current length of m_pEnv + size_t m_EnvLen; // Current length of m_pEnv autodeps_t m_AutoDeps; set< const fileinfo* , less_fileinfo > m_Targets; // List of targets that are build by this makefile @@ -363,7 +363,7 @@ public: mh_pid_t EchoCommand(const string &Params) const; string SearchCommand(const string &Command, const string &Extension="") const; const string &GetPythonExe() const; - int SearchPath(const char *szCommand, const char *pExt, int Len, char *szFullCommand,char **pFilePart) const; + int SearchPath(const char *szCommand, const char *pExt, size_t Len, char *szFullCommand,char **pFilePart) const; mh_pid_t OsExeCommand(const string &Command, const string &Params, bool IgnoreError, string *pOutput) const; }; diff --git a/tools/mhmake/src/util.h b/tools/mhmake/src/util.h index 8a2058539..6e8e7c9eb 100644 --- a/tools/mhmake/src/util.h +++ b/tools/mhmake/src/util.h @@ -50,7 +50,7 @@ #define PLATFORM "linux" #endif -#define MHMAKEVER "2.2.3" +#define MHMAKEVER "2.2.4" class makecommand { -- cgit v1.2.3