ROX-26763: Enhance the handling of failures in procfsscraper

collector/lib/ProcfsScraper.cpp

@@ -2,6 +2,7 @@
 
 #include <cctype>
 #include <cinttypes>
+#include <cstdio>
 #include <cstring>
 #include <fcntl.h>
 #include <string_view>
@@ -143,6 +144,26 @@ bool GetSocketINodes(int dirfd, uint64_t pid, UnorderedSet<SocketInfo>* sock_ino
   return true;
 }
 
+// IsZombieProcess fetches the current state of the process pointed to by dirfd, and
+// - returns true, if the process state is 'Z'
+// - returns false otherwise
+bool IsZombieProcess(int dirfd) {
+  FileHandle stat_file(FDHandle(openat(dirfd, "stat", O_RDONLY)), "r");
+  if (!stat_file.valid()) {
+    return false;
+  }
+
+  char linebuf[512];
+
+  if (fgets(linebuf, sizeof(linebuf), stat_file.get()) == nullptr) {
+    return false;
+  }
+
+  auto state = ExtractProcessState(linebuf);
+
+  return state && *state == 'Z';
+}
+
 // GetContainerID retrieves the container ID of the process represented by dirfd. The container ID is extracted from
 // the cgroup.
 std::optional<std::string> GetContainerID(int dirfd) {
@@ -491,14 +512,17 @@ bool ReadContainerConnections(const char* proc_path, std::shared_ptr<ProcessStor
       continue;
     }
 
+    if (IsZombieProcess(dirfd)) {
+      continue;
+    }
+
     auto container_id = GetContainerID(dirfd);
     if (!container_id) {
       continue;
     }
 
     uint64_t netns_inode;
     if (!GetNetworkNamespace(dirfd, &netns_inode)) {
-      // TODO ROX-13962: Improve logging to indicate when a process is defunct.
       COUNTER_INC(CollectorStats::procfs_could_not_get_network_namespace);
       CLOG_THROTTLED(ERROR, std::chrono::seconds(10)) << "Could not determine network namespace: " << StrError();
       continue;
@@ -609,6 +633,22 @@ std::optional<std::string_view> ExtractContainerID(std::string_view cgroup_line)
   return ExtractContainerIDFromCgroup(cgroup_path);
 }
 
+std::optional<char> ExtractProcessState(std::string_view line) {
+  size_t last_parenthese;
+
+  if ((last_parenthese = line.rfind(") ")) == line.npos) {
+    return {};
+  }
+
+  line.remove_prefix(last_parenthese + 2);
+
+  if (line.empty()) {
+    return {};
+  }
+
+  return line[0];
+}
+
 bool ConnScraper::Scrape(std::vector<Connection>* connections, std::vector<ContainerEndpoint>* listen_endpoints) {
   return ReadContainerConnections(proc_path_.c_str(), process_store_, connections, listen_endpoints);
 }

collector/lib/ProcfsScraper_internal.h

@@ -9,6 +9,11 @@ namespace collector {
 // ExtractContainerID tries to extract a container ID from a cgroup line.
 std::optional<std::string_view> ExtractContainerID(std::string_view cgroup_line);
 
+// ExtractProcessState retrieves the state of the process (3rd element).
+// as found in /proc/<pid>/stat.
+// Returns: the state character or nullopt in case of error.
+std::optional<char> ExtractProcessState(std::string_view proc_pid_stat_line);
+
 }  // namespace collector
 
 #endif

collector/test/ConnScraperTest.cpp

@@ -46,6 +46,27 @@ TEST(ConnScraperTest, TestExtractContainerID) {
   }
 }
 
+TEST(ConnScraperTest, TestProcStateExtract) {
+  std::optional<char> state;
+
+  // valid line
+  state = ExtractProcessState("13934 (prog) Z 2312 13934 2312 34819 13934 4194304 94 0 0 0 0 0 0 0 20 0 1 0 608787 5758976 409 18446744073709551615 94201870180352 94201870200233 140728860702192 0 0 0 0 0 0 0 0 0 17 4 0 0 0 0 0 94201870216240 94201870217856 94202687545344 140728860710184 140728860710204 140728860710204 140728860712939 0\n");
+
+  EXPECT_TRUE(state);
+  EXPECT_EQ(*state, 'Z');
+
+  // invalid
+  state = ExtractProcessState("13934 (prog) ");
+
+  EXPECT_FALSE(state);
+
+  // program name containing ')'
+  state = ExtractProcessState("13934 (prog ) Z) R 2312 13934 2312 34819 13934 4194304 94 0 0 0 0 0 0 0 20 0 1 0 608787 5758976 409 18446744073709551615 94201870180352 94201870200233 140728860702192 0 0 0 0 0 0 0 0 0 17 4 0 0 0 0 0 94201870216240 94201870217856 94202687545344 140728860710184 140728860710204 140728860710204 140728860712939 0\n");
+
+  EXPECT_TRUE(state);
+  EXPECT_EQ(*state, 'R');
+}
+
 }  // namespace
 
 }  // namespace collector