Solved reentrancy problem in commandqueue

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<fileinfo> > 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<activeentry>[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<activeentry>[NrParallelBuilds];

 }

 

-void commandqueue::ThrowCommandExecutionError(activeentry *pActiveEntry)

+void commandqueue::ThrowCommandExecutionError(refptr<activeentry> pActiveEntry)

 {

   refptr<fileinfo> 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::activeentry> commandqueue::CreateActiveEntry(void)

 {

-//cout << "Adding entry "<<m_NrActiveEntries<<" to queue:"<<ActiveEntry.pTarget->GetQuotedFullFileName()<<" ("<<ActiveProcess<<")\n";

-  m_pActiveEntries[m_NrActiveEntries]=ActiveEntry;

-  m_pActiveProcesses[m_NrActiveEntries]=ActiveProcess;

+  refptr<activeentry> pRet=new activeentry;

+  m_pActiveEntries[m_NrActiveEntries]=pRet;

+  m_pActiveProcesses[m_NrActiveEntries]=this->m_DummyWaitHandle;

   m_NrActiveEntries++;

+  return pRet;

+}

+

+unsigned commandqueue::GetActiveEntryId(const refptr<activeentry> pActiveEntry) const

+{

+  unsigned i=0;

+  for (i=0; i<m_NrActiveEntries; i++)

+    if (m_pActiveEntries[i]==pActiveEntry)

+      return i;

+  throw("ActiveEntry not found for "+ pActiveEntry->pTarget->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<activeentry> pActiveEntry, mh_pid_t *pActiveProcess)

 {

   refptr<fileinfo> 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<activeentry> pActiveEntry)

 {

   refptr<fileinfo> pTarget=pActiveEntry->pTarget;

 

@@ -237,20 +254,20 @@ bool commandqueue::StartExecuteCommands(const refptr<fileinfo> &pTarget)
   mhmakeparser *pMakefile=pRule->GetMakefile();

   vector<string>::iterator CommandIt=pRule->GetCommands().begin();

 

-  activeentry ActiveEntry;

+  refptr<activeentry> 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<fileinfo> &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<fileinfo> &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<activeentry> pActiveEntry=m_pActiveEntries[Ret];

     refptr<fileinfo> pCurrentTarget=pActiveEntry->pTarget;

     refptr<rule> 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<fileinfo>               pTarget;

     vector<string>::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<fileinfo> >  m_Queue;

   unsigned                   m_MaxNrCommandsInParallel;

   mh_pid_t                  *m_pActiveProcesses;

-  activeentry               *m_pActiveEntries;

+  refptr<activeentry>       *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<activeentry> pActiveEntry);

+  refptr<activeentry> CreateActiveEntry(void);

+  unsigned GetActiveEntryId(const refptr<activeentry> pActiveEntry) const;

   void RemoveActiveEntry(unsigned Entry);

+  void RemoveActiveEntry(refptr<activeentry> pActiveEntry)

+  {

+    RemoveActiveEntry(GetActiveEntryId(pActiveEntry));

+  }

   bool StartExecuteCommands(const refptr<fileinfo> &pTarget);

-  bool StartExecuteNextCommand(activeentry *pActiveEntry, mh_pid_t *pActiveProcess);

-  void TargetBuildFinished(activeentry *pActiveEntry);

+  bool StartExecuteNextCommand(refptr<activeentry> pActiveEntry, mh_pid_t *pActiveProcess);

+  void TargetBuildFinished(refptr<activeentry> 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

 {