zturner created this revision.
zturner added reviewers: clayborg, labath, amccarth.
zturner added a subscriber: lldb-commits.

For seemingly no reason today, I started getting crashes on exit in 
`CleanupProcessSpecificTempDirectory` when I ran certain tests.  I determined 
this wasn't related to a code change, and when I looked in my temp lldb 
directory, I had close to 1500 process specific temp directories that had never 
been cleaned up, some dating back over 1 year ago.

Best I can figure out is that in the case of running the test suite, we've 
installed an atexit handler for the python executable itself.  Which means it 
will call this atexit handler after it has already (presumably) unloaded our 
extension module, leading to undefined behavior.

Whatever the cause, atexit handlers are dangerous because of the restrictions 
on what kind of code you can run in them.  I moved this to an explicit cleanup 
on `Terminate`, and verified that there are no more crashes on exit, and 
additionally the process specific temp directory is now correctly cleaned up 
after every test.

I don't know how (or even if) this was ever working on other platforms besides 
Windows, but you guys should check your temp/lldb directory and see if there 
are thousands of directories in there.

http://reviews.llvm.org/D17420

Files:
  include/lldb/Host/HostInfoBase.h
  source/Host/common/HostInfoBase.cpp
  source/Initialization/SystemInitializerCommon.cpp

Index: source/Initialization/SystemInitializerCommon.cpp
===================================================================
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -197,6 +197,7 @@
 #endif
     OperatingSystemGo::Terminate();
 
+    HostInfoBase::Terminate();
     Log::Terminate();
 
 #if defined(_MSC_VER)
Index: source/Host/common/HostInfoBase.cpp
===================================================================
--- source/Host/common/HostInfoBase.cpp
+++ source/Host/common/HostInfoBase.cpp
@@ -82,6 +82,14 @@
     g_fields = new HostInfoBaseFields();
 }
 
+void
+HostInfoBase::Terminate()
+{
+    CleanupProcessSpecificLLDBTempDir();
+    delete g_fields;
+    g_fields = nullptr;
+}
+
 uint32_t
 HostInfoBase::GetNumberCPUS()
 {
@@ -335,9 +343,6 @@
     if (!FileSystem::MakeDirectory(temp_file_spec, 
eFilePermissionsDirectoryDefault).Success())
         return false;
 
-    // Make an atexit handler to clean up the process specify LLDB temp dir
-    // and all of its contents.
-    ::atexit(CleanupProcessSpecificLLDBTempDir);
     file_spec.GetDirectory().SetCString(temp_file_spec.GetCString());
     return true;
 }
Index: include/lldb/Host/HostInfoBase.h
===================================================================
--- include/lldb/Host/HostInfoBase.h
+++ include/lldb/Host/HostInfoBase.h
@@ -34,6 +34,8 @@
 
   public:
     static void Initialize();
+    static void
+    Terminate();
 
     //------------------------------------------------------------------
     /// Returns the number of CPUs on this current host.


Index: source/Initialization/SystemInitializerCommon.cpp
===================================================================
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -197,6 +197,7 @@
 #endif
     OperatingSystemGo::Terminate();
 
+    HostInfoBase::Terminate();
     Log::Terminate();
 
 #if defined(_MSC_VER)
Index: source/Host/common/HostInfoBase.cpp
===================================================================
--- source/Host/common/HostInfoBase.cpp
+++ source/Host/common/HostInfoBase.cpp
@@ -82,6 +82,14 @@
     g_fields = new HostInfoBaseFields();
 }
 
+void
+HostInfoBase::Terminate()
+{
+    CleanupProcessSpecificLLDBTempDir();
+    delete g_fields;
+    g_fields = nullptr;
+}
+
 uint32_t
 HostInfoBase::GetNumberCPUS()
 {
@@ -335,9 +343,6 @@
     if (!FileSystem::MakeDirectory(temp_file_spec, eFilePermissionsDirectoryDefault).Success())
         return false;
 
-    // Make an atexit handler to clean up the process specify LLDB temp dir
-    // and all of its contents.
-    ::atexit(CleanupProcessSpecificLLDBTempDir);
     file_spec.GetDirectory().SetCString(temp_file_spec.GetCString());
     return true;
 }
Index: include/lldb/Host/HostInfoBase.h
===================================================================
--- include/lldb/Host/HostInfoBase.h
+++ include/lldb/Host/HostInfoBase.h
@@ -34,6 +34,8 @@
 
   public:
     static void Initialize();
+    static void
+    Terminate();
 
     //------------------------------------------------------------------
     /// Returns the number of CPUs on this current host.
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to