[Lldb-commits] [PATCH] D145136: Add a Debugger interruption mechanism in parallel to the Command Interpreter interruption

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I kinda like this.



================
Comment at: lldb/include/lldb/Core/Debugger.h:563
+  // the previous IOHandler thread.
+  const HostThread SetIOHandlerThread(HostThread &new_thread);
 
----------------
`const` on a return value rarely makes sense. OTOH, `const` on the argument usually does. Did you want this the other way around?


================
Comment at: lldb/include/lldb/Core/Debugger.h:647
+  uint32_t m_interrupt_requested = 0;
+  std::vector<std::string> m_interrupt_stack;
+  std::recursive_mutex m_interrupt_mutex;
----------------
I think this is unused


================
Comment at: lldb/include/lldb/Core/Debugger.h:648
+  std::vector<std::string> m_interrupt_stack;
+  std::recursive_mutex m_interrupt_mutex;
 
----------------
can we make this non-recursive?


================
Comment at: lldb/source/API/SBDebugger.cpp:1700
+}
+  void SBDebugger::CancelInterruptRequest()  {
+  LLDB_INSTRUMENT_VA(this);
----------------
bad formatting


================
Comment at: lldb/test/API/python_api/was_interrupted/TestDebuggerInterruption.py:39
+        def rendevous(self):
+            # We smuggle out lock and event to the runner thread using thread local data:
+            import interruptible
----------------
This had me confused for a while, because "thread local data" has a very specific meaning. Could you rename this to something else?


================
Comment at: lldb/test/API/python_api/was_interrupted/TestDebuggerInterruption.py:112
+        if not "check" in args:
+            self.lock = threading.Lock()
+            self.event = threading.Event()
----------------
I am confused as to what this lock's purpose is. I can see that it's taken on one thread and released on another (which is in itself an unusual thing to do, even though python permits it), but I still don't understand what does it achieve. Lock are generally used for protecting a certain object. If the goal is to control the interleaving of two threads, then maybe a sequence of barriers would be better. Something like:
```
#thread 1
work1()
t1_done.wait()
t2_done.wait()
work3()
t1_done.wait()
# etc.. odd-numbered work happens on this thread

#thread 2
t1_done.wait()
work2()
t2_done.wait()
t1_done.wait()
work4()
# etc.. even-numbered work happens here
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145136/new/

https://reviews.llvm.org/D145136