labath marked 2 inline comments as done.
labath added inline comments.

================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:376-386
+    bool handled = llvm::any_of(m_processes, [&](NativeProcessLinux *process) {
+      return process->TryHandleWaitStatus(pid, status);
+    });
+    if (!handled) {
+      if (status.type == WaitStatus::Stop && status.status == SIGSTOP) {
+        // Store the thread for later collection.
+        m_unowned_threads.insert(pid);
----------------
yinghuitan wrote:
> Maybe I misunderstood the purpose -- the code stores `pid` into 
> `m_unowned_threads` for later collection if **none** of the process handle 
> it. Instead, I thought we want to store into `m_unowned_threads` if **at 
> least one process** did not handle it? 
> 
> Can you add some comment to clarify the intention? Thanks.
The code is correct. Each event should be handled by exactly one process, but 
it may not happen immediately (if the process doesn't know about the thread). 
I've added some comments to clarify.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:538
+      (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal)) {
+    SetExitStatus(status, true);
+    return true;
----------------
yinghuitan wrote:
> Can you add back the original comment which I find useful? Thanks
> ```
> // The process exited.  We're done monitoring.  Report to delegate.
> ```
Yeah sure. It got lost because I changed the code a couple of times while 
working on it.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h:70-72
+    llvm::SmallPtrSet<NativeProcessLinux *, 2> m_processes;
+
+    llvm::DenseSet<::pid_t> m_unowned_threads;
----------------
yinghuitan wrote:
> I assume no lock is needed? 
Yes. This is all single-threaded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146977

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits]... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... jeffrey tan via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... jeffrey tan via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Nico Weber via Phabricator via lldb-commits
    • [Lldb-com... Felipe de Azevedo Piovezan via Phabricator via lldb-commits
    • [Lldb-com... Pavel Labath via Phabricator via lldb-commits

Reply via email to