libutil/cgroup: ensure that cleanup takes place even under interruptions

When Ctrl-C is sent to the workload, even across remote builds, the
whole process possess a global flag `_isInterrupted` which is checked in
certain filesystem operations, cancelling them, e.g. writeFile will
write nothing under interruption unwinding.

In addition, if any operation throws an exception before we `rmdir` the
cgroup, we may leave it hanging while we remove the state record.
Therefore, we put the final cleanup in a block.

In practice, reading statistics could lead to failures.

Control groups cleanups are critical though and should always be
performed.

Change-Id: I48fa87317b6a9f6663559bc8fa5f8a897f37011e
Signed-off-by: Raito Bezarius <raito@lix.systems>

lix/libutil/cgroup.cc

@@ -50,10 +50,9 @@ static std::map<std::string, std::string> getCgroups(const Path & cgroupFile)
     return cgroups;
 }
 
-static CgroupStats readStatistics(const std::filesystem::path & cgroup)
-{
+static Result<CgroupStats> readStatistics(const std::filesystem::path & cgroup)
+try {
     CgroupStats stats;
-
     auto cpustatPath = cgroup / "cpu.stat";
 
     if (pathExists(cpustatPath)) {
@@ -77,13 +76,15 @@ static CgroupStats readStatistics(const std::filesystem::path & cgroup)
     }
 
     return stats;
+} catch (...) {
+    return result::current_exception();
 }
 
 static void killCgroup(const std::string & name, const std::filesystem::path & cgroup)
 {
     auto killFile = cgroup / "cgroup.kill";
     if (pathExists(killFile))
-        writeFile(killFile, "1");
+        writeFileUninterruptible(killFile, "1");
     else {
         throw SysError(
             "cgroup '%s' at '%s' does not possess `cgroup.kill` ; are you running Lix on a kernel "
@@ -115,7 +116,7 @@ destroyCgroup(const std::string & name, const std::filesystem::path & aliveCgrou
 
     killCgroup(name, aliveCgroup);
 
-    CgroupStats stats = readStatistics(aliveCgroup);
+    Result<CgroupStats> stats = readStatistics(aliveCgroup);
 
     if (rmdir(aliveCgroup.c_str()) == -1) {
         throw SysError("deleting cgroup '%s' at '%s'", name, aliveCgroup);
@@ -123,7 +124,8 @@ destroyCgroup(const std::string & name, const std::filesystem::path & aliveCgrou
 
     debug("cgroup '%s' destroyed", name);
 
-    return stats;
+    /* Even if this contains an exception, this is fine. */
+    return stats.value();
 }
 
 CgroupHierarchy getLocalHierarchy(const std::filesystem::path & cgroupFilesystem)
@@ -376,7 +378,7 @@ CgroupStats AutoDestroyCgroup::getStatistics() const
      */
     return std::visit(
         nix::overloaded{
-            [&](const Path & aliveCgroup) { return readStatistics(aliveCgroup); },
+            [&](const Path & aliveCgroup) { return readStatistics(aliveCgroup).value(); },
             [&](const CgroupStats & stats) { return stats; }
         },
         cgroup_

lix/libutil/file-system.cc

@@ -358,14 +358,13 @@ Generator<Bytes> readFileSource(const Path & path)
     }(std::move(fd));
 }
 
-
-void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync)
+void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync, bool allowInterrupts)
 {
     AutoCloseFD fd{open(path.c_str(), O_WRONLY | O_TRUNC | O_CREAT | O_CLOEXEC, mode)};
     if (!fd)
         throw SysError("opening file '%1%'", path);
     try {
-        writeFull(fd.get(), s);
+        writeFull(fd.get(), s, allowInterrupts);
     } catch (Error & e) {
         e.addTrace({}, "writing file '%1%'", path);
         throw;
@@ -378,6 +377,10 @@ void writeFile(const Path & path, std::string_view s, mode_t mode, bool sync)
         syncParent(path);
 }
 
+void writeFileUninterruptible(const Path & path, std::string_view s, mode_t mode, bool sync)
+{
+    writeFile(path, s, mode, sync, false);
+}
 
 static AutoCloseFD openForWrite(const Path & path, mode_t mode)
 {

lix/libutil/file-system.hh

@@ -189,7 +189,16 @@ Generator<Bytes> readFileSource(const Path & path);
 /**
  * Write a string to a file.
  */
-void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false);
+void writeFile(
+    const Path & path,
+    std::string_view s,
+    mode_t mode = 0666,
+    bool sync = false,
+    bool allowInterrupts = true
+);
+void writeFileUninterruptible(
+    const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false
+);
 
 void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false);
 kj::Promise<Result<void>>

tests/nixos/cgroups/default.nix

@@ -25,6 +25,7 @@
 
     # Start build in background
     host.execute("NIX_REMOTE=daemon nix build --use-cgroups --auto-allocate-uids --file ${./hang.nix} >&2 &")
+    pid = int(host.succeed("pgrep nix"))
     service = "/sys/fs/cgroup/system.slice/nix-daemon.service"
 
     # Wait for cgroups to be created
@@ -35,6 +36,13 @@
     host.succeed(f'[ -z "$(cat {service}/cgroup.procs)" ]')
     host.succeed(f'[ -n "$(cat {service}/supervisor/cgroup.procs)" ]')
     host.succeed(f'[ -n "$(cat {service}/nix-build-uid-*/cgroup.procs)" ]')
+
+    # Perform an interrupt
+    host.execute(f"kill -SIGINT {pid}")
+
+    # Check that there aren't any cgroups anymore, neither any state records
+    host.succeed(f"until [ ! -e {service}/nix-build-uid-* ]; do sleep 1; done", timeout=30)
+    host.succeed("until [ ! -e /nix/var/nix/cgroups/nix-build-uid-* ]; do sleep 1; done", timeout=30)
   '';
 
 }