Page MenuHomePhabricator

Fix debugger shutdown when Python interpreter is loaded
ClosedPublic

Authored by ovyalov on Sep 7 2015, 6:47 PM.

Details

Reviewers
clayborg
Summary

Python locks in memory a few global objects like lldb.debugger,lldb.target,... - as a consequence, ~Debugger isn't called upon shutdown.
Calling Debugger::Clear ensures that ScriptInterpreterPython::Clear is called to clean up Python global variables.

Diff Detail

Event Timeline

ovyalov updated this revision to Diff 34176.Sep 7 2015, 6:47 PM
ovyalov retitled this revision from to Fix debugger shutdown when Python interpreter is loaded.
ovyalov updated this object.
ovyalov added a reviewer: clayborg.
ovyalov added a subscriber: lldb-commits.
zturner added a subscriber: zturner.Sep 7 2015, 7:00 PM

What were the symptoms of this? How'd you find it?

What were the symptoms of this? How'd you find it?

I was debugging port forwarding with PlatformAndroid - in particular, whether forwarded ports are cleaned up with ~PlatformAndroidRemoteGDBServer. Turned out, that if there was at least one script command executed - platforms' destructors were never called.

tfiala added a subscriber: tfiala.Sep 7 2015, 10:04 PM
tfiala added inline comments.
source/Core/Debugger.cpp
426

Wouldn't Clear() be considered a mutating function? So a const debugger ref seems like maybe it should be non-const?

i.e.
for (auto& debugger: debuggers) {

debugger->Clear();

}

ovyalov added inline comments.Sep 7 2015, 10:27 PM
source/Core/Debugger.cpp
426

Clear is mutable, but debugger has DebuggerSP type here - so, const is applicable only to shared_ptr, not to Debugger instance itself.

tfiala added inline comments.Sep 7 2015, 10:46 PM
source/Core/Debugger.cpp
426

Ah gotcha.

clayborg accepted this revision.Sep 8 2015, 9:11 AM
clayborg edited edge metadata.

Looks good.

This revision is now accepted and ready to land.Sep 8 2015, 9:11 AM
ovyalov closed this revision.Sep 8 2015, 9:34 AM

Files:

/lldb/trunk/source/Core/Debugger.cpp

Users:

ovyalov (Author)

http://reviews.llvm.org/rL247023