Hi there,
I have glanced over your patch
I do not have a Windows box to try and repro these shutdown issues, so I might
very well be missing some critical Windows-specific detail in my feedback
Release() seems to be a fairly dangerous accessor - indeed I am not positive
that releasing without deallocation the session and modules dictionaries will
do the right thing when Py_Finalize() is not involved
I might very well create an SBDebugger (which - via the command interpreter -
owns the ScriptInterpreterPython); if I simply let go of the last reference to
the SBDebugger, the destruction chain should do the right thing, and I expect
your release-without-decref calls might cause some objects to survive their due
lifetime
At the very least, would it be possible to wrap those releases in an if
(Py_IsInitialized()) like you do in another change?
As for 3 - nothing in the Python documentation specifies that the atexit
facility does not allow any “meaningful code” execution; the main caveat I
could find online about it is that there are escape routes out of Python that
atexit() does not handle - but that seems a very different concern from what
you are seeing
Even if we actually end up deciding that atexit is not to be used, your patch
as-is has eliminated the call to the code to delete crashinfo.so - that is not
critical of course, but it worries me to see a duplication of
lldb.SBDebugger.Terminate() calls - it might be more appropriate, if anything,
to define a function
def exitFromTestSuite(exitCode = None):
deleteCrashInfoDylib(dylib_dst)
lldb.SBDebugger.Terminate()
if exitCode != None:
sys.exit(exitCode)
Then we would all know to put our atexit calls in exitFromTestSuite(), and we’d
have some degree of confidence that all cleanup tasks will be executed on all
ways out of the test suite.
But, again, I am not sure why atexit is giving you woes - that would probably
be worth looking into
> On Aug 7, 2014, at 4:03 PM, Zachary Turner <[email protected]> wrote:
>
> 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.
>
> http://reviews.llvm.org/D4826
>
> Files:
> include/lldb/Interpreter/PythonDataObjects.h
> source/Interpreter/ScriptInterpreterPython.cpp
> test/dotest.py
> <D4826.12296.patch>_______________________________________________
> lldb-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
- Enrico
📩 egranata@.com ☎️ 27683
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
