From 226f86fea9bbf479294333c0bc11d55977f6557b Mon Sep 17 00:00:00 2001
From: Johannes Schauer Marin Rodrigues <josch@mister-muffin.de>
Date: Tue, 30 Aug 2022 21:55:57 +0200
Subject: [PATCH] fix mmdebstrap hanging if apt in download step failed
 (closes: #1017795)

---
 coverage.txt                 |  2 +
 mmdebstrap                   | 81 +++++++++++++++++++++---------------
 tests/variant-custom-timeout | 11 +++++
 3 files changed, 60 insertions(+), 34 deletions(-)
 create mode 100644 tests/variant-custom-timeout

diff --git a/coverage.txt b/coverage.txt
index 290f076..88ac992 100644
--- a/coverage.txt
+++ b/coverage.txt
@@ -323,3 +323,5 @@ Modes: root unshare
 Needs-QEMU: true
 
 Test: error-if-stdout-is-tty
+
+Test: variant-custom-timeout
diff --git a/mmdebstrap b/mmdebstrap
index 9894a9e..fb7c412 100755
--- a/mmdebstrap
+++ b/mmdebstrap
@@ -979,46 +979,59 @@ sub run_apt_download_progress {
     my $flags = fcntl($wfh, F_GETFD, 0)        or error "fcntl F_GETFD: $!";
     fcntl($wfh, F_SETFD, $flags & ~FD_CLOEXEC) or error "fcntl F_SETFD: $!";
     my $fd = fileno $wfh;
-    # 2022-05-02, #debian-apt on OFTC, times in UTC+2
-    # 16:57 < josch> DonKult: how is -oDebug::pkgDpkgPm=1 -oDir::Log=/dev/null
-    #                a "fancy no-op"?
-    # 11:52 < DonKult> josch: "fancy no-op" in sofar as it does nothing to the
-    #                  system even through its not in a special mode ala
-    #                  simulation or download-only. It does all the things it
-    #                  normally does, except that it just prints the dpkg calls
-    #                  instead of execv() them which in practice amounts means
-    #                  it does nothing (the Dir::Log just prevents libapt from
-    #                  creating the /var/log/apt directories. As the code
-    #                  creates them even if no logs will be placed there…). As
-    #                  said, midterm an apt --print-install-packages or
-    #                  something would be nice to avoid running everything.
-    run_apt_progress({
-            ARGV => [
-                'apt-get',
-                '--yes',
-                '-oDebug::pkgDpkgPm=1',
-                '-oDir::Log=/dev/null',
-                $options->{dryrun}
-                ? '-oAPT::Get::Simulate=true'
-                : (
-                    "-oAPT::Keep-Fds::=$fd",
-                    "-oDPkg::Tools::options::'cat >&$fd'::InfoFD=$fd",
-                    "-oDpkg::Pre-Install-Pkgs::=cat >&$fd",
-                    # no need to lock the database if we are just downloading
-                    "-oDebug::NoLocking=1",
-                    # no need for pty magic if we write no log
-                    "-oDpkg::Use-Pty=0",
-                ),
-                @{ $options->{APT_ARGV} },
-            ],
-        });
+    # run_apt_progress() can raise an exception which would leave this function
+    # without cleaning up the other thread we started, making mmdebstrap hang
+    # in case run_apt_progress() fails -- so wrap this in eval() instead
+    eval {
+        # 2022-05-02, #debian-apt on OFTC, times in UTC+2
+        # 16:57 < josch> DonKult: how is -oDebug::pkgDpkgPm=1
+        #                -oDir::Log=/dev/null a "fancy no-op"?
+        # 11:52 < DonKult> josch: "fancy no-op" in sofar as it does nothing to
+        #                  the system even through its not in a special mode
+        #                  ala simulation or download-only. It does all the
+        #                  things it normally does, except that it just prints
+        #                  the dpkg calls instead of execv() them which in
+        #                  practice amounts means it does nothing (the Dir::Log
+        #                  just prevents libapt from creating the /var/log/apt
+        #                  directories. As the code creates them even if no
+        #                  logs will be placed there…). As said, midterm an apt
+        #                  --print-install-packages or something would be nice
+        #                  to avoid running everything.
+        run_apt_progress({
+                ARGV => [
+                    'apt-get',
+                    '--yes',
+                    '-oDebug::pkgDpkgPm=1',
+                    '-oDir::Log=/dev/null',
+                    $options->{dryrun}
+                    ? '-oAPT::Get::Simulate=true'
+                    : (
+                        "-oAPT::Keep-Fds::=$fd",
+                        "-oDPkg::Tools::options::'cat >&$fd'::InfoFD=$fd",
+                        "-oDpkg::Pre-Install-Pkgs::=cat >&$fd",
+                      # no need to lock the database if we are just downloading
+                        "-oDebug::NoLocking=1",
+                        # no need for pty magic if we write no log
+                        "-oDpkg::Use-Pty=0",
+                    ),
+                    @{ $options->{APT_ARGV} },
+                ],
+            });
+    };
+    my $err = '';
+    if ($@) {
+        $err = "apt download failed: $@";
+    }
     # signal the child process that we are done
     close $wfh;
     # and then read from it what it got
     my @listofdebs = <$fh>;
     close $fh;
     if ($? != 0) {
-        error "status child failed";
+        $err = "status child failed";
+    }
+    if ($err) {
+        error $err;
     }
     # remove trailing newlines
     chomp @listofdebs;
diff --git a/tests/variant-custom-timeout b/tests/variant-custom-timeout
new file mode 100644
index 0000000..06f0b42
--- /dev/null
+++ b/tests/variant-custom-timeout
@@ -0,0 +1,11 @@
+#!/bin/sh
+set -eu
+export LC_ALL=C.UTF-8
+
+# mmdebstrap used to hang forever if apt in custom mode failed to resolve
+# dependencies because a process died without cleaning up its children.
+# https://bugs.debian.org/1017795
+ret=0
+{{ CMD }} --mode={{ MODE }} --variant=custom \
+	--include=this-package-does-not-exist {{ DIST }} /dev/null {{ MIRROR }} || ret=1
+[ $ret -eq 1 ]