Page MenuHomePhabricator

Fixup some issues with Python shutdown / finalization

Authored by zturner on Aug 7 2014, 4:03 PM.



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.
  1. 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()
  1. 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.

Diff Detail


Event Timeline

zturner updated this revision to Diff 12296.Aug 7 2014, 4:03 PM
zturner retitled this revision from to Fixup some issues with Python shutdown / finalization.
zturner updated this object.
zturner edited the test plan for this revision. (Show Details)
zturner added a subscriber: Unknown Object (MLST).
zturner updated this revision to Diff 12310.Aug 8 2014, 11:19 AM

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

zturner closed this revision.Aug 8 2014, 4:29 PM
zturner updated this revision to Diff 12316.

Closed by commit rL215256 (authored by @zturner).