[Lldb-commits] [PATCH] Fixup some issues with Python shutdown / finalization

[Zachary Turner]

Sounds good.  I will make that change, and then leave a comment that we
should figure out why (or if) atexit is problematic.  It might turn out
that it's actually fine, and I will probably end up looking at it again in
the future after I get to the point that the entire test suite can run
start to end on Windows without anything crashing.


On Fri, Aug 8, 2014 at 3:12 PM, Enrico Granata <egranata at apple.com> wrote:

> Ah, well, my bad then - I didn’t realize I had never filled in that
> function!
>
> It would be nice to get a more detailed look at what the issue is,
> definitely - I am somewhat busy these days so I won’t get a chance to
> figure this one out for a while; if you’re in the zone enough that you want
> to pursue this deeper, be my guest :)
> If not, I would say, let’s just make a function
>
> def exitTestSuite(exitCode = None):
>   lldb.SBDebugger.Terminate()
>   if exitCode: sys.exit(exitCode)
>
> and then always call exitTestSuite() instead of duplicating the
> Terminate() and exit() calls in several places (if we want to do a third
> thing on exit, then we know we only have one place to edit instead of
> several)
>
> Once that is addressed, LGTM and you should feel free to commit
>
> On Aug 8, 2014, at 2:59 PM, Zachary Turner <zturner at google.com> wrote:
>
> I just didn't revert the changes to dotest.py.  I do agree that it would
> be nice to figure out what, if any problem there is with atexit(), but TBH
> my python (and even moreso my embedded python) is not the strongest, so I'm
> kind of handicapped in that regard, and I also think it just makes for
> easier debugging and easier to rationalize about the code-flow without
> them.  Regarding your original comment about the proposed patch removing
> the cleanup for the crashinfo.so, that atexit handler was actually not even
> doing anything (empty function).
>
> Let me know if you want me to look into this more though.
>
>
> On Fri, Aug 8, 2014 at 1:23 PM, Enrico Granata <egranata at apple.com> wrote:
>
>> Do we still get crashes because of the usage of atexit(), or did you
>> simply not revert the changes to dotest.py?
>>
>> With the exception of those, the rest of your patch LGTM
>>
>> On Aug 8, 2014, at 11:19 AM, Zachary Turner <zturner at google.com> wrote:
>>
>> Got rid of the Release() method and wrapped the destructor-based decrefs
>> inside of Py_IsInitialized() checks.
>>
>> http://reviews.llvm.org/D4826
>>
>> Files:
>>  include/lldb/Interpreter/PythonDataObjects.h
>>  include/lldb/Interpreter/ScriptInterpreterPython.h
>>  source/Interpreter/ScriptInterpreterPython.cpp
>>  test/dotest.py
>> <D4826.12310.patch>_______________________________________________
>>
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>
>>
>>  *- Enrico*
>> 📩 egranata@.com ☎️ 27683
>>
>>
>>
>>
>
> *- Enrico*
> 📩 egranata@.com ☎️ 27683
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140808/5521b8d0/attachment.html>