[Lldb-commits] [PATCH] D146977: [lldb-server/linux] Use waitpid(-1) to collect inferior events

[jeffrey tan via Phabricator via lldb-commits]

yinghuitan 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);
----------------
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.


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:538
+      (status.type == WaitStatus::Exit || status.type == WaitStatus::Signal)) {
+    SetExitStatus(status, true);
+    return true;
----------------
Can you add back the original comment which I find useful? Thanks
```
// The process exited.  We're done monitoring.  Report to delegate.
```


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


================
Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.h:72
+
+    llvm::DenseSet<::pid_t> m_unowned_threads;
+
----------------
Add a comment for this field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146977