reaper: don't reap unpopulated cgroups that still have child cgroups

Fixes a race condition that led to an endless loop.

Author: MaxKellermann

debian/changelog

@@ -2,6 +2,7 @@ cm4all-spawn (0.26) unstable; urgency=low
 
   * reaper: schedule EPOLLPRI on cgroup.events
   * reaper: reduce number of system calls while scanning cgroup tree
+  * reaper: don't reap unpopulated cgroups that still have child cgroups
   * lua: new libraries:
     - "control_client
     - "socket"

src/reaper/TreeWatch.cxx

@@ -261,8 +261,12 @@ TreeWatch::HandleDeletedDirectory(Directory &parent,
 
 	HandleDeletedDirectory(child);
 
-	if (!child.persist)
+	if (!child.persist) {
 		parent.children.erase(i);
+
+		if (parent.children.empty())
+			OnDirectoryEmpty(parent.GetRelativePath());
+	}
 }
 
 inline void

src/reaper/TreeWatch.hxx

@@ -76,6 +76,14 @@ public:
 			: FileDescriptor::Undefined();
 	}
 
+	[[gnu::pure]]
+	bool IsDirectoryEmpty(std::string_view relative_path) const noexcept {
+		const auto *directory = FindDirectory(relative_path);
+		return directory != nullptr
+			? directory->children.empty()
+			: true;
+	}
+
 private:
 	/**
 	 * Look up a #Directory object.  Returns nullptr if the
@@ -110,5 +118,6 @@ protected:
 
 	virtual void OnDirectoryCreated(std::string_view relative_path,
 					FileDescriptor directory_fd) noexcept = 0;
+	virtual void OnDirectoryEmpty(std::string_view relative_path) noexcept = 0;
 	virtual void OnDirectoryDeleted(std::string_view relative_path) noexcept = 0;
 };

src/reaper/UnifiedWatch.cxx

@@ -114,6 +114,13 @@ UnifiedCgroupWatch::ReAddCgroup(std::string_view relative_path) noexcept
 void
 UnifiedCgroupWatch::OnGroupEmpty(Group &group) noexcept
 {
+	if (!IsDirectoryEmpty(group.GetRelativePath()))
+		/* there are still child cgroups, but they are
+		   unpopulated; they may be populated soon, so don't
+		   reap this cgroup yet; later, OnDirectoryEmpty() may
+		   do that */
+		return;
+
 	callback(("/" + group.GetRelativePath()).c_str());
 
 	auto i = groups.find(group.GetRelativePath());
@@ -224,6 +231,23 @@ UnifiedCgroupWatch::OnDirectoryCreated(const std::string_view relative_path,
 	}
 }
 
+void
+UnifiedCgroupWatch::OnDirectoryEmpty(std::string_view relative_path) noexcept
+{
+	auto i = groups.find(relative_path);
+	if (i == groups.end())
+		return;
+
+	auto &group = i->second;
+	if (group.IsPopulated())
+		/* the last child cgroup was deleted, but this one is
+		   still populated, so don't reap it */
+		return;
+
+	callback(("/" + group.GetRelativePath()).c_str());
+	groups.erase(i);
+}
+
 void
 UnifiedCgroupWatch::OnDirectoryDeleted(const std::string_view relative_path) noexcept
 {

src/reaper/UnifiedWatch.hxx

@@ -56,5 +56,6 @@ protected:
 	bool ShouldSkipName(std::string_view name) const noexcept override;
 	void OnDirectoryCreated(std::string_view relative_path,
 				FileDescriptor directory_fd) noexcept override;
+	void OnDirectoryEmpty(std::string_view relative_path) noexcept override;
 	void OnDirectoryDeleted(std::string_view relative_path) noexcept override;
 };

test/RunTreeWatch.cxx

@@ -28,6 +28,10 @@ class MyTreeWatch final : public TreeWatch {
 		fmt::print("+ {}\n", relative_path);
 	}
 
+	void OnDirectoryEmpty(std::string_view relative_path) noexcept override {
+		fmt::print("e {}\n", relative_path);
+	}
+
 	void OnDirectoryDeleted(std::string_view relative_path) noexcept override {
 		fmt::print("- {}\n", relative_path);
 	}