<div dir="ltr">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.</div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Aug 8, 2014 at 3:12 PM, Enrico Granata <span dir="ltr"><<a href="mailto:egranata@apple.com" target="_blank">egranata@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Ah, well, my bad then - I didn’t realize I had never filled in that function!</div>
<div><br></div><div>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 :)</div>
<div>If not, I would say, let’s just make a function</div><div><br></div><div>def exitTestSuite(exitCode = None):</div><div> lldb.SBDebugger.Terminate()</div><div> if exitCode: sys.exit(exitCode)</div><div><br></div><div>
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)</div>
<div><br></div><div>Once that is addressed, LGTM and you should feel free to commit</div><div><div class="h5"><br><div><blockquote type="cite"><div>On Aug 8, 2014, at 2:59 PM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:</div>
<br><div><div dir="ltr">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).<div>
<br></div><div>Let me know if you want me to look into this more though. </div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Aug 8, 2014 at 1:23 PM, Enrico Granata <span dir="ltr"><<a href="mailto:egranata@apple.com" target="_blank">egranata@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div>Do we still get crashes because of the usage of atexit(), or did you simply not revert the changes to dotest.py?</div>
<div><br></div><div>With the exception of those, the rest of your patch LGTM</div><br><div><blockquote type="cite"><div><div><div>On Aug 8, 2014, at 11:19 AM, Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>> wrote:</div>
<br></div></div><div><div><div>Got rid of the Release() method and wrapped the destructor-based decrefs inside of Py_IsInitialized() checks.<br><br><a href="http://reviews.llvm.org/D4826" target="_blank">http://reviews.llvm.org/D4826</a><br>
<br>Files:<br> include/lldb/Interpreter/PythonDataObjects.h<br> include/lldb/Interpreter/ScriptInterpreterPython.h<br> source/Interpreter/ScriptInterpreterPython.cpp<br> test/dotest.py<br></div></div><span><D4826.12310.patch></span>_______________________________________________<div>
<br>lldb-commits mailing list<br><a href="mailto:lldb-commits@cs.uiuc.edu" target="_blank">lldb-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits</a><br>
</div></div></blockquote></div><div><br><div>
<div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<div><i>- Enrico</i><br>📩 egranata@<font color="#ff2600"></font>.com ☎️ 27683</div><div><br></div></div></div></div></div><br>
</div>
<br></div></div></blockquote></div><br></div>
</div></blockquote></div><br><div>
<div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word"><div style="color:rgb(0,0,0);letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;word-wrap:break-word">
<div><i>- Enrico</i><br>📩 egranata@<font color="#ff2600"></font>.com ☎️ 27683</div><div><br></div></div></div></div></div><br>
</div>
<br></div></div></div></blockquote></div><br></div>