Re: [Lldb-commits] [PATCH] D12683: Fix debugger shutdown when Python interpreter is loaded

[Oleksiy Vyalov via lldb-commits]

ovyalov closed this revision.
ovyalov added a comment.

Files:

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

Users:

  ovyalov (Author)

http://reviews.llvm.org/rL247023


http://reviews.llvm.org/D12683



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12683: Fix debugger shutdown when Python interpreter is loaded

[Todd Fiala via lldb-commits]

tfiala added inline comments.


Comment at: source/Core/Debugger.cpp:426
@@ +425,3 @@
+for (const auto& debugger: debuggers)
+debugger->Clear();
+

ovyalov wrote:
> tfiala wrote:
> > 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();
> > }
> > 
> Clear is mutable, but debugger has DebuggerSP type here - so, const is 
> applicable only to shared_ptr, not to Debugger instance itself.
Ah gotcha.


http://reviews.llvm.org/D12683



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12683: Fix debugger shutdown when Python interpreter is loaded

[Zachary Turner via lldb-commits]

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

On Mon, Sep 7, 2015 at 6:47 PM Oleksiy Vyalov via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> ovyalov created this revision.
> ovyalov added a reviewer: clayborg.
> ovyalov added a subscriber: lldb-commits.
>
> 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.
>
> http://reviews.llvm.org/D12683
>
> Files:
>   source/Core/Debugger.cpp
>
> Index: source/Core/Debugger.cpp
> ===
> --- source/Core/Debugger.cpp
> +++ source/Core/Debugger.cpp
> @@ -421,7 +421,11 @@
>
>  // Clear our master list of debugger objects
>  Mutex::Locker locker (GetDebuggerListMutex ());
> -GetDebuggerList().clear();
> +auto& debuggers = GetDebuggerList();
> +for (const auto& debugger: debuggers)
> +debugger->Clear();
> +
> +debuggers.clear();
>  }
>
>  void
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D12683: Fix debugger shutdown when Python interpreter is loaded

[Oleksiy Vyalov via lldb-commits]

ovyalov created this revision.
ovyalov added a reviewer: clayborg.
ovyalov added a subscriber: lldb-commits.

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. 

http://reviews.llvm.org/D12683

Files:
  source/Core/Debugger.cpp

Index: source/Core/Debugger.cpp
===
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -421,7 +421,11 @@
 
 // Clear our master list of debugger objects
 Mutex::Locker locker (GetDebuggerListMutex ());
-GetDebuggerList().clear();
+auto& debuggers = GetDebuggerList();
+for (const auto& debugger: debuggers)
+debugger->Clear();
+
+debuggers.clear();
 }
 
 void


Index: source/Core/Debugger.cpp
===
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -421,7 +421,11 @@
 
 // Clear our master list of debugger objects
 Mutex::Locker locker (GetDebuggerListMutex ());
-GetDebuggerList().clear();
+auto& debuggers = GetDebuggerList();
+for (const auto& debugger: debuggers)
+debugger->Clear();
+
+debuggers.clear();
 }
 
 void
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D12683: Fix debugger shutdown when Python interpreter is loaded

[Oleksiy Vyalov via lldb-commits]

ovyalov added inline comments.


Comment at: source/Core/Debugger.cpp:426
@@ +425,3 @@
+for (const auto& debugger: debuggers)
+debugger->Clear();
+

tfiala wrote:
> 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();
> }
> 
Clear is mutable, but debugger has DebuggerSP type here - so, const is 
applicable only to shared_ptr, not to Debugger instance itself.


http://reviews.llvm.org/D12683



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits