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

[Enrico Granata]

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 <mailto: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 <mailto: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 <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 <mailto:lldb-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits <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/c678a9be/attachment.html>