Fixup some issues with Python shutdown / finalization

[zturner]

Summary

There were a number of issues related to python shutdown. These issues appear to be present on every platform, but are benign except for Windows. On Windows they present because Windows must build against a debug python interpreter, which asserts when these issues arise. The issues, along with their respective fixes, are:

1. decrementing the reference count of dead objects. This happens because we store the session dictionary in the ScriptInterpreterPython class, and when these are destroyed on shutdown, they get decref'ed. However, this happens during or after a Py_Finalize, so Python has already destroyed these objects. The fix employed here is to add a Release() method to PythonObject() which simply drops the underlying object.

2. Attempting to execute embedded python during Py_Finalize. This results in error messages "NameError: module 'lldb' not found" after tests run. The fix here is to only execute these commands if Py_IsInitialized()

3. Use of sys.atexit(). atexit handlers are run as part of Py_Finalize, and at this point it is unspecified which, if any modules have been destroyed. So no meaningful code can run here. The fix employed here is to explicitly call lldb.SBDebugger.Terminate() before calling sys.exit(). A better fix would be to not use sys.exit() at all, but instead have a clean shutdown path, perhaps by way of raising an exception that is caught at the top-level of the python script.

[zturner]

Got rid of the Release() method and wrapped the destructor-based decrefs inside of Py_IsInitialized() checks.

[zturner]

Closed by commit rL215256 (authored by @zturner).

include/lldb/Interpreter/PythonDataObjects.h

public:
             return Reset(object.get());
         }
 
         virtual bool
         Reset (PyObject* py_obj = NULL)
         {
             if (py_obj != m_py_obj)
             {
+                if (Py_IsInitialized())
                     Py_XDECREF(m_py_obj);
                 m_py_obj = py_obj;
+                if (Py_IsInitialized())
                     Py_XINCREF(m_py_obj);
             }
             return true;
         }
         
         void
         Dump () const
         {
             if (m_py_obj)

include/lldb/Interpreter/ScriptInterpreterPython.h

public:
         {
             return m_object && m_object != Py_None;
         }
         
         
         virtual
         ~ScriptInterpreterPythonObject()
         {
+            if (Py_IsInitialized())
                 Py_XDECREF(m_object);
             m_object = NULL;
         }
         private:
             DISALLOW_COPY_AND_ASSIGN (ScriptInterpreterPythonObject);
     };
 public:
   class Locker : public ScriptInterpreterLocker
   {

source/Interpreter/ScriptInterpreterPython.cpp

 void
 ScriptInterpreterPython::Clear ()
 {
     // Release any global variables that might have strong references to
     // LLDB objects when clearing the python script interpreter.
     Locker locker(this,
                   ScriptInterpreterPython::Locker::AcquireLock,
                   ScriptInterpreterPython::Locker::FreeAcquiredLock);
+
+    // This may be called as part of Py_Finalize.  In that case the modules are destroyed in random
+    // order and we can't guarantee that we can access these.
+    if (Py_IsInitialized())
         PyRun_SimpleString("lldb.debugger = None; lldb.target = None; lldb.process = None; lldb.thread = None; lldb.frame = None");
 }
 
 bool
 ScriptInterpreterPython::BreakpointCallbackFunction 
 (
     void *baton,
     StoppointCallbackContext *context,
     user_id_t break_id,

test/dotest.py

 
 Type:
 
 ./dotest.py -h
 
 for available options.
 """
 
-import atexit
 import commands
 import os
 import errno
 import platform
 import progress
 import signal
 import subprocess
 import sys
if platform.system() == "Darwin":
         test_dir = os.environ['LLDB_TEST']
         if not test_dir or not os.path.exists(test_dir):
             return
         dylib_src = os.path.join(test_dir,"crashinfo.c")
         dylib_dst = os.path.join(test_dir,"crashinfo.so")
         cmd = "xcrun clang %s -o %s -framework Python -Xlinker -dylib -iframework /System/Library/Frameworks/ -Xlinker -F /System/Library/Frameworks/" % (dylib_src,dylib_dst)
         if subprocess.call(cmd,shell=True) == 0 and os.path.exists(dylib_dst):
             setCrashInfoHook = setCrashInfoHook_Mac
-            atexit.register(deleteCrashInfoDylib,dylib_dst)
     else:
         pass
 
 def parseOptionsAndInitTestdirs():
     """Initialize the list of directories containing our unittest scripts.
 
     '-h/--help as the first option prints out usage info and exit the program.
     """
for testdir in testdirs:
     os.path.walk(testdir, visit, 'Test')
 
 #
 # Now that we have loaded all the test cases, run the whole test suite.
 #
 
 # For the time being, let's bracket the test runner within the
 # lldb.SBDebugger.Initialize()/Terminate() pair.
-import lldb, atexit
-# Update: the act of importing lldb now executes lldb.SBDebugger.Initialize(),
-# there's no need to call it a second time.
-#lldb.SBDebugger.Initialize()
-atexit.register(lambda: lldb.SBDebugger.Terminate())
+import lldb
 
 # Create a singleton SBDebugger in the lldb namespace.
 lldb.DBG = lldb.SBDebugger.Create()
 
 if lldb_platform_name:
     print "Setting up remote platform '%s'" % (lldb_platform_name)
     lldb.remote_platform = lldb.SBPlatform(lldb_platform_name)
     if not lldb.remote_platform.IsValid():
         print "error: unable to create the LLDB platform named '%s'." % (lldb_platform_name)
+        lldb.SBDebugger.Terminate()
         sys.exit(1)
     if lldb_platform_url:
         # We must connect to a remote platform if a LLDB platform URL was specified
         print "Connecting to remote platform '%s' at '%s'..." % (lldb_platform_name, lldb_platform_url)
         platform_connect_options = lldb.SBPlatformConnectOptions(lldb_platform_url); 
         err = lldb.remote_platform.ConnectRemote(platform_connect_options)
         if err.Success():
             print "Connected."
         else:
             print "error: failed to connect to remote platform using URL '%s': %s" % (lldb_platform_url, err)
+            lldb.SBDebugger.Terminate()
             sys.exit(1)
     
     if lldb_platform_working_dir:
         print "Setting remote platform working directory to '%s'..." % (lldb_platform_working_dir)
         lldb.remote_platform.SetWorkingDirectory(lldb_platform_working_dir)
     
     lldb.remote_platform_working_dir = lldb_platform_working_dir
     lldb.DBG.SetSelectedPlatform(lldb.remote_platform)
else:
                     compilers[i] = cmd_output.split('\n')[0]
                     print "'xcrun -find %s' returning %s" % (c, compilers[i])
 
 if not parsable:
     print "compilers=%s" % str(compilers)
 
 if not compilers or len(compilers) == 0:
     print "No eligible compiler found, exiting."
+    lldb.SBDebugger.Terminate()
     sys.exit(1)
 
 if isinstance(compilers, list) and len(compilers) >= 1:
     iterCompilers = True
 
 # Make a shallow copy of sys.path, we need to manipulate the search paths later.
 # This is only necessary if we are relocated and with different configurations.
 if rdir:

 
 # Terminate the test suite if ${LLDB_TESTSUITE_FORCE_FINISH} is defined.
 # This should not be necessary now.
 if ("LLDB_TESTSUITE_FORCE_FINISH" in os.environ):
     print "Terminating Test suite..."
     subprocess.Popen(["/bin/sh", "-c", "kill %s; exit 0" % (os.getpid())])
 
 # Exiting.
-sys.exit(failed)
+lldb.SBDebugger.Terminate()