Fixing bug related to incorrect unloading of shared library and class code.
authoraszostak <aszostak@f7af4fe6-9843-0410-8265-dc069ae4e863>
Sun, 8 Nov 2009 22:21:19 +0000 (22:21 +0000)
committeraszostak <aszostak@f7af4fe6-9843-0410-8265-dc069ae4e863>
Sun, 8 Nov 2009 22:21:19 +0000 (22:21 +0000)
This is related to:
https://savannah.cern.ch/bugs/?58083
https://savannah.cern.ch/bugs/?58247
In actual fact this is a workaround for the bug/feature/limitation (whatever you want to call it) in ROOT/CINT that removes more libraries than just the one that was
requested to be removed. The problem has been solved by adding more protection to prevent the segfault, printing of appropriate warnings and cleaning up HLT components in
the correct order to prevent the problem in the first place.

HLT/BASE/AliHLTHOMERLibManager.cxx
HLT/BASE/AliHLTHOMERLibManager.h
HLT/BASE/AliHLTSystem.cxx
HLT/sim/AliHLTOUTComponent.cxx
HLT/trigger/AliHLTGlobalTriggerComponent.cxx
HLT/trigger/AliHLTGlobalTriggerComponent.h
HLT/trigger/AliHLTGlobalTriggerWrapper.cxx

index 8707160..b2a81f2 100644 (file)
 #include "AliHLTHOMERLibManager.h"
 #include "AliHLTHOMERReader.h"
 #include "AliHLTHOMERWriter.h"
+#include "AliHLTLogging.h"
 #include "TString.h"
 #include "TSystem.h"
 
 /** ROOT macro for the implementation of ROOT specific class methods */
 ClassImp(AliHLTHOMERLibManager)
 
+// This list must be NULL terminated, since we use it as a marker to identify
+// the end of the list.
+const char* AliHLTHOMERLibManager::fgkLibraries[] = {"libAliHLTHOMER.so", "libHOMER.so", NULL};
+// The size of the list of reference counts must be one less than fgkLibraries.
+int AliHLTHOMERLibManager::fgkLibRefCount[] = {0, 0};
+
 AliHLTHOMERLibManager::AliHLTHOMERLibManager()
   :
   fLibraryStatus(0),
@@ -46,7 +53,8 @@ AliHLTHOMERLibManager::AliHLTHOMERLibManager()
   fFctCreateReaderFromBuffer(NULL),
   fFctDeleteReader(NULL),
   fFctCreateWriter(NULL),
-  fFctDeleteWriter(NULL)
+  fFctDeleteWriter(NULL),
+  fLoadedLib(NULL)
 {
   // see header file for class documentation
   // or
@@ -58,6 +66,7 @@ AliHLTHOMERLibManager::AliHLTHOMERLibManager()
 AliHLTHOMERLibManager::~AliHLTHOMERLibManager()
 {
   // see header file for class documentation
+  UnloadHOMERLibrary();
 }
 
 AliHLTHOMERReader* AliHLTHOMERLibManager::OpenReader(const char* hostname, unsigned short port )
@@ -164,16 +173,23 @@ int AliHLTHOMERLibManager::LoadHOMERLibrary()
 {
   // see header file for class documentation
   int iResult=-EBADF;
-  const char* libraries[]={"libAliHLTHOMER.so", "libHOMER.so", NULL};
-  const char** library=&libraries[0];
+  const char** library=&fgkLibraries[0];
+  int* refcount = &fgkLibRefCount[0];
   do {
     TString libs = gSystem->GetLibraries();
-    if (libs.Contains(*library) ||
-       (gSystem->Load(*library)) >= 0) {
+    if (libs.Contains(*library)) {
+      iResult=1;
+      break;
+    }
+    if ((gSystem->Load(*library)) >= 0) {
+      ++(*refcount);
+      fLoadedLib = *library;
       iResult=1;
       break;
     }
-  } while (*(++library)!=NULL);
+    ++library;
+    ++refcount;
+  } while ((*library)!=NULL);
 
   if (iResult>0 && *library!=NULL) {
     // print compile info
@@ -217,3 +233,71 @@ int AliHLTHOMERLibManager::LoadHOMERLibrary()
 
   return iResult;
 }
+
+int AliHLTHOMERLibManager::UnloadHOMERLibrary()
+{
+  // see header file for class documentation
+  int iResult=0;
+  
+  if (fLoadedLib != NULL)
+  {
+    // Find the corresponding reference count.
+    const char** library=&fgkLibraries[0];
+    int* refcount = &fgkLibRefCount[0];
+    while (*library != NULL)
+    {
+      if (strcmp(*library, fLoadedLib) == 0) break;
+      ++library;
+      ++refcount;
+    }
+    
+    // Decrease the reference count and remove the library if it is zero.
+    if (*refcount >= 0) --(*refcount);
+    if (*refcount == 0)
+    {
+      // Check that the library we are trying to unload is actually the last library
+      // in the gSystem->GetLibraries() list. If not then we must abort the removal.
+      // This is because of a ROOT bug/feature/limitation. If we try unload the library
+      // then ROOT will also wipe all libraries in the gSystem->GetLibraries() list
+      // following the library we want to unload.
+      TString libstring = gSystem->GetLibraries();
+      TString token, lastlib;
+      Ssiz_t from = 0;
+      Int_t numOfLibs = 0, posOfLib = -1;
+      while (libstring.Tokenize(token, from, " "))
+      {
+        ++numOfLibs;
+        lastlib = token;
+        if (token.Contains(fLoadedLib)) posOfLib = numOfLibs;
+      }
+      if (numOfLibs == posOfLib)
+      {
+        gSystem->Unload(fLoadedLib);
+
+        // Check that the library is gone, since Unload() does not return a status code.
+        libstring = gSystem->GetLibraries();
+        if (libstring.Contains(fLoadedLib)) iResult = -EBADF;
+      }
+      else
+      {
+        AliHLTLogging log;
+        log.LoggingVarargs(kHLTLogWarning, Class_Name(), FUNCTIONNAME(), __FILE__, __LINE__,
+          Form("ROOT limitation! Cannot properly cleanup and unload the shared"
+            " library '%s' since another library '%s' was loaded afterwards. Trying to"
+            " unload this library will remove the others and lead to serious memory faults.",
+            fLoadedLib, lastlib.Data()
+        ));
+      }
+    }
+  }
+
+  // Clear the function pointers.
+  fFctCreateReaderFromTCPPort = NULL;
+  fFctCreateReaderFromTCPPorts = NULL;
+  fFctCreateReaderFromBuffer = NULL;
+  fFctDeleteReader = NULL;
+  fFctCreateWriter = NULL;
+  fFctDeleteWriter = NULL;
+
+  return iResult;
+}
index 1cff197..96e940d 100644 (file)
@@ -134,6 +134,11 @@ class AliHLTHOMERLibManager {
    */
   int LoadHOMERLibrary();
 
+  /**
+   * Unloads the HOMER library.
+   */
+  int UnloadHOMERLibrary();
+
   /** status of the loading of the HOMER library */
   int fLibraryStatus; //!transient
 
@@ -155,6 +160,12 @@ class AliHLTHOMERLibManager {
   /** entry in the HOMER library */
   void (*fFctDeleteWriter)(); //!transient
 
+  /** Indicates the library that was actually (and if) loaded in LoadHOMERLibrary(). */
+  const char* fLoadedLib;  //!transient
+
+  static const char* fgkLibraries[]; /// List of libraries to try and load.
+  static int fgkLibRefCount[]; /// The library reference count to control when to unload the library.
+
   ClassDef(AliHLTHOMERLibManager, 0)
 };
 #endif
index 645bf57..b2fcbfc 100644 (file)
@@ -739,7 +739,7 @@ int AliHLTSystem::DeinitTasks()
 {
   // see header file for class documentation
   int iResult=0;
-  TObjLink *lnk=fTaskList.FirstLink();
+  TObjLink *lnk=fTaskList.LastLink();
   while (lnk) {
     TObject* obj=lnk->GetObject();
     if (obj) {
@@ -751,7 +751,7 @@ int AliHLTSystem::DeinitTasks()
 //       HLTInfo("task %s cleaned (%d), current memory usage %d %d", pTask->GetName(), iResult, ProcInfo.fMemResident, ProcInfo.fMemVirtual);
     } else {
     }
-    lnk = lnk->Next();
+    lnk = lnk->Prev();
   }
   fEventCount=-1;
   fGoodEvents=-1;
index 457bc9e..a97038a 100644 (file)
@@ -135,6 +135,14 @@ int AliHLTOUTComponent::DoInit( int argc, const char** argv )
   if (iResult>=0) {
   }
 
+  // Make sure there is no library manager before we try and create a new one.
+  if (fpLibManager) {
+    delete fpLibManager;
+    fpLibManager=NULL;
+  }
+  
+  // Create a new library manager and allocate the appropriate number of
+  // HOMER writers for the HLTOUT component.
   fpLibManager=new AliHLTHOMERLibManager;
   if (fpLibManager) {
     int writerNo=0;
@@ -168,17 +176,18 @@ int AliHLTOUTComponent::DoDeinit()
       // wanted to have a dynamic_cast<AliHLTHOMERWriter*> here, but this results into
       // undefined symbol when loading the library
       if (*element!=NULL) {
-       // this ia a quick fix for bug https://savannah.cern.ch/bugs/?58247
-       // it is unclear why the pointer is not valid any more, for more
-       // details: https://savannah.cern.ch/bugs/?58083
-       //(*element)->Clear();
-       //fpLibManager->DeleteWriter((AliHLTHOMERWriter*)(*element));
+       (*element)->Clear();
+       fpLibManager->DeleteWriter((AliHLTHOMERWriter*)(*element));
       } else {
        HLTError("writer instance is NULL");
       }
       element=fWriters.erase(element);
     }
   }
+  if (fpLibManager) {
+    delete fpLibManager;
+    fpLibManager=NULL;
+  }
 
   if (fpDigitTree) {
     delete fpDigitTree;
index bb490b4..fff1758 100644 (file)
@@ -38,7 +38,7 @@
 #include "TRegexp.h"
 #include "TClonesArray.h"
 #include "TObjString.h"
-#include "TSystem.h"
+#include "TString.h"
 #include "TInterpreter.h"
 #include "TDatime.h"
 #include "TClass.h"
@@ -78,7 +78,8 @@ AliHLTGlobalTriggerComponent::AliHLTGlobalTriggerComponent() :
        fBufferSizeConst(2*(sizeof(AliHLTGlobalTriggerDecision) + sizeof(AliHLTReadoutList))),
        fBufferSizeMultiplier(1.),
        fIncludePaths(TObjString::Class()),
-       fIncludeFiles(TObjString::Class())
+       fIncludeFiles(TObjString::Class()),
+       fLibStateAtLoad()
 {
   // Default constructor.
   
@@ -1127,6 +1128,9 @@ int AliHLTGlobalTriggerComponent::LoadTriggerClass(
   }
   else
   {
+    // Store the library state to be checked later in UnloadTriggerClass.
+    fLibStateAtLoad = gSystem->GetLibraries();
+    
     // If we do not support the compiler then try interpret the class instead.
     TString cmd = ".L ";
     cmd += filename;
@@ -1155,12 +1159,41 @@ int AliHLTGlobalTriggerComponent::UnloadTriggerClass(const char* filename)
   TString compiler = gSystem->GetBuildCompilerVersion();
   if (fRuntimeCompile && (compiler.Contains("gcc") or compiler.Contains("icc")))
   {
+    // Generate the library name.
     TString libname = filename;
     Ssiz_t dotpos = libname.Last('.');
     if (0 <= dotpos and dotpos < libname.Length()) libname[dotpos] = '_';
     libname += ".";
     libname += gSystem->GetSoExt();
     
+    // This is a workaround for a problem with unloading shared libraries in ROOT.
+    // If the trigger logic library is loaded before the libAliHLTHOMER.so library
+    // or any other library is loaded afterwards, then during the gInterpreter->UnloadFile
+    // call all the subsequent libraries get unloded. This means that any objects created
+    // from classes implemented in the libAliHLTHOMER.so library will generate segfaults
+    // since the executable code has been unloaded.
+    // We need to check if there are any more libraries loaded after the class we
+    // are unloading and in that case don't unload the class.
+    TString libstring = gSystem->GetLibraries();
+    TString token, lastlib;
+    Ssiz_t from = 0;
+    Int_t numOfLibs = 0, posOfLib = -1;
+    while (libstring.Tokenize(token, from, " "))
+    {
+      ++numOfLibs;
+      lastlib = token;
+      if (token.Contains(libname)) posOfLib = numOfLibs;
+    }
+    if (numOfLibs != posOfLib)
+    {
+      HLTWarning(Form("ROOT limitation! Cannot properly cleanup and unload the shared"
+          " library '%s' since another library '%s' was loaded afterwards. Trying to"
+          " unload this library will remove the others and lead to serious memory faults.",
+          libname.Data(), lastlib.Data()
+      ));
+      return 0;
+    }
+    
     char* path = NULL;
     int result = 0;
     if ((path = gSystem->DynamicPathName(libname)) != NULL)
@@ -1172,7 +1205,30 @@ int AliHLTGlobalTriggerComponent::UnloadTriggerClass(const char* filename)
   }
   else
   {
-    // If we do not support the compiler then try interpret the class instead.
+    // This is again a workaround for the problem with unloading files in ROOT.
+    // If the trigger logic class is loaded before the libAliHLTHOMER.so library
+    // or any other library is loaded afterwards, then during the gInterpreter->UnloadFile
+    // call all the subsequent libraries get unloded.
+    // We need to check if the list of loaded libraries has changed since the last
+    // call to LoadTriggerClass. If it has then don't unload the class.
+    if (fLibStateAtLoad != gSystem->GetLibraries())
+    {
+      TString libstring = gSystem->GetLibraries();
+      TString token;
+      Ssiz_t from = 0;
+      while (libstring.Tokenize(token, from, " "))
+      {
+        if (not fLibStateAtLoad.Contains(token)) break;
+      }
+      HLTWarning(Form("ROOT limitation! Cannot properly cleanup and unload the file"
+          " '%s' since another library '%s' was loaded afterwards. Trying to unload"
+          " this file will remove the other library and lead to serious memory faults.",
+          filename, token.Data()
+      ));
+      return 0;
+    }
+  
+    // If we did not compile the trigger logic then remove the interpreted class.
     TString cmd = ".U ";
     cmd += filename;
     Int_t errorcode = TInterpreter::kNoError;
index f5fd852..6a18f29 100644 (file)
@@ -260,6 +260,7 @@ class AliHLTGlobalTriggerComponent : public AliHLTTrigger
   double fBufferSizeMultiplier; //! Buffer size multiplier estimate for GetOutputDataSize.
   TClonesArray fIncludePaths; //! Paths specified by the -includepath command line option.
   TClonesArray fIncludeFiles; //! Files specified by the -include command line option.
+  TString fLibStateAtLoad; //! This stores the loaded libraries just before we tell CINT to load the interpreted file.
 
   static const char* fgkTriggerMenuCDBPath; //! The path string to read the trigger menu from the CDB.
   
index 8a59060..db797af 100644 (file)
 #include "TArrayL64.h"
 #include "TClass.h"
 #include "TInterpreter.h"
-
-#ifdef R__BUILDING_CINT7
-#include "cint7/Api.h"
-#else
-#include "Api.h"
-#endif
+#include "TCint.h"
 
 ClassImp(AliHLTGlobalTriggerWrapper)
 
@@ -132,7 +127,7 @@ AliHLTGlobalTriggerWrapper::AliHLTGlobalTriggerWrapper(const char* classname) :
   fFuncPtr = AliHLTOnErrorInCINT;
   if (sizeof(fPtr) == sizeof(fFuncPtr))
   {
-    G__set_errmsgcallback(fPtr);
+    gInterpreter->SetErrmsgcallback(fPtr);
   }
   else
   {