[Lldb-commits] [lldb] [lldb] Fix race in macOS's FindProcessesImpl (PR #204109)

[Jonas Devlieghere via lldb-commits]

================
@@ -684,34 +685,59 @@ static bool 
GetMacOSXProcessUserAndGroup(ProcessInstanceInfo &process_info) {
   return false;
 }
 
-uint32_t Host::FindProcessesImpl(const ProcessInstanceInfoMatch &match_info,
-                                 ProcessInstanceInfoList &process_infos) {
-  std::vector<struct kinfo_proc> kinfos;
-
+/// Fetches the list of all processes. Returns true on success.
+[[nodiscard]] static bool
+GetProcessList(std::vector<struct kinfo_proc> &kinfos) {
   int mib[3] = {CTL_KERN, KERN_PROC, KERN_PROC_ALL};
 
-  size_t pid_data_size = 0;
-  if (::sysctl(mib, 3, nullptr, &pid_data_size, nullptr, 0) != 0)
-    return 0;
+  // This is an inherently racy API. We have to first query the size for our
+  // buffer and then pass back to sysctl. If more processes spawn between the
+  // size query and the actual call to fetch, then sysctl returns ENOMEM.
+  // We keep retrying until we get a passing result.
+  for (int attempt = 1; attempt < 1024; ++attempt) {
+    // Fetch the buffer size sysctl would return.
+    size_t current_pid_size = 0;
+    if (::sysctl(mib, 3, nullptr, &current_pid_size, nullptr, 0) != 0)
+      return false;
 
-  // Add a few extra in case a few more show up
-  const size_t estimated_pid_count =
-      (pid_data_size / sizeof(struct kinfo_proc)) + 10;
+    // We expect some process growth. Start small and keep increasing the
+    // expected growth each attempt until we can get the sysctl call to
+    // succeed.
+    const size_t expected_growth = attempt * 128 * sizeof(struct kinfo_proc);
+    const size_t estimated_pid_size = current_pid_size + expected_growth;
 
-  kinfos.resize(estimated_pid_count);
-  pid_data_size = kinfos.size() * sizeof(struct kinfo_proc);
+    // Allocate the buffer sysctl will copy the process list into.
+    kinfos.resize(estimated_pid_size / sizeof(struct kinfo_proc));
 
-  if (::sysctl(mib, 3, &kinfos[0], &pid_data_size, nullptr, 0) != 0)
-    return 0;
+    // Fetch the actual process list and let syctl adjust actual_pid_size.
+    size_t actual_pid_size = kinfos.size() * sizeof(struct kinfo_proc);
+    if (::sysctl(mib, 3, &kinfos[0], &actual_pid_size, nullptr, 0) == 0) {
+      // Adjust the buffer to the actual number of processes returned.
+      kinfos.resize(actual_pid_size / sizeof(struct kinfo_proc));
+      return true;
+    }
 
-  const size_t actual_pid_count = (pid_data_size / sizeof(struct kinfo_proc));
+    // Errno is set to ENOMEM if our estimated_pid_size is too small.
+    if (errno != ENOMEM)
+      return false;
+  }
+
+  return false;
+}
+
+uint32_t Host::FindProcessesImpl(const ProcessInstanceInfoMatch &match_info,
+                                 ProcessInstanceInfoList &process_infos) {
+  std::vector<struct kinfo_proc> kinfos;
+  if (!GetProcessList(kinfos)) {
+    Log *log(GetLog(LLDBLog::Host | LLDBLog::Process));
+    LLDB_LOG(log, "error: Failed to read process list.");
----------------
JDevlieghere wrote:

```suggestion
    LLDB_LOG(GetLog(LLDBLog::Host | LLDBLog::Process), "error: Failed to read 
process list.");
```

https://github.com/llvm/llvm-project/pull/204109
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits