Merge ~sergiodj/ubuntu/+source/squid3:squid-bug1761096-dnsmasq-hangs-restart into ubuntu/+source/squid3:ubuntu/xenial-devel

[Sergio Durigan Junior]

This is a fix (more like a workaround) for bug #1761096.

This bug required a decent amount of investigative effort from both Christian and I, and even though we were unable to find the exact root cause for it, I'm pretty confident that the right way to go is to incorporate this proposed workaround in order to mitigate the issue.

When using dnsmasq along with squid on Ubuntu Xenial, the user will experience a deadlock while performing on every second execution of the "systemctl start dnsmasq.service" command. The deadlock will be caused by an attempt to invoke, by nss-lookup.target, a "systemctl reload squid.service", which will itself try to start the nss-lookup.target, which will be waiting on squid, therefore eventually leading to a timeout.

The underlying cause of this deadlock is related to how systemd used to handle dependencies between multiple jobs being started in the same transaction.

One can reproduce the issue on a Xenial container:

$ lxc launch ubuntu-daily:xenial squid-bug1761096
$ lxc shell squid-bug1761096
# apt update
# apt install squid dnsmasq -y

It is quite possible that during "apt install" the bug will manifest, and dnsmasq will fail to start due to a timeout. The user might see a message like:

Job for dnsmasq.service failed because a timeout was exceeded. See "systemctl status dnsmasq.service" and "journalctl -xe" for details.

If the bug doesn't manifest itself during installation, the following commands in sequence should trigger it:

# systemctl restart dnsmasq.service
# systemctl restart dnsmasq.service

There is a PPA with the proposed change here:

https://launchpad.net/~sergiodj/+archive/ubuntu/squid3-bug1761096

autopkgtest is still happy:

autopkgtest [09:43:08]: @@@@@@@@@@@@@@@@@@@@ summary
squid PASS

[Sergio Durigan Junior (sergiodj)]

I'm adding Christian to the list of reviewers because he helped with the investigation.

[Christian Ehrhardt  (paelzer)]

Thanks Sergio for converting our thoughts into an MP.

I think this is a nice tradeoff between "ignoring valid, but low importance (rare) user problem" and "spending way too much extra time debugging it further".

The change works in all our experiments, will only change xenial (as later on it doesn't trigger the same way anymore) and t me looks like a low regression risk.

I hope the SRU Team agrees but my +1 you have.

review: Approve

[Sergio Durigan Junior (sergiodj)]

On Monday, September 07 2020, Christian Ehrhardt  wrote:

> Thanks Sergio for converting our thoughts into an MP.
>
> I think this is a nice tradeoff between "ignoring valid, but low importance (rare) user problem" and "spending way too much extra time debugging it further".
>
> The change works in all our experiments, will only change xenial (as later on it doesn't trigger the same way anymore) and t me looks like a low regression risk.
>
> I hope the SRU Team agrees but my +1 you have.

Thanks for the review, Christian.

Could you also sponsor the package for me, please?

Cheers,

--
Sergio
GPG key ID: E92F D0B3 6B14 F1F4 D8E0 EB2F 106D A1C8 C3CB BF14

[Christian Ehrhardt  (paelzer)]

To ssh://git.launchpad.net/~usd-import-team/ubuntu/+source/squid3
 * [new tag] upload/3.5.12-1ubuntu7.14 -> upload/3.5.12-1ubuntu7.14

Uploading to ubuntu (via ftp to upload.ubuntu.com):
  Uploading squid3_3.5.12-1ubuntu7.14.dsc: done.
  Uploading squid3_3.5.12-1ubuntu7.14.debian.tar.xz: done.
  Uploading squid3_3.5.12-1ubuntu7.14_source.buildinfo: done.
  Uploading squid3_3.5.12-1ubuntu7.14_source.changes: done.
Successfully uploaded packages.

diff --git a/debian/changelog b/debian/changelog
index ef58e12..85ada66 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,14 @@
+squid3 (3.5.12-1ubuntu7.14) xenial; urgency=medium
+
+  * d/squid.resolvconf: Invoke "systemctl reload --no-block" if we are
+    using systemd.  This prevents squid from blocking if the reload
+    action is being issued indirectly because of another
+    service (e.g., because dnsmasq has been restarted), which may
+    cause a deadlock and prevent the whole transaction to
+    complete. (LP: #1761096)
+
+ -- Sergio Durigan Junior <[e-mail]>  Fri, 04 Sep 2020 08:31:36 -0400
+
 squid3 (3.5.12-1ubuntu7.13) xenial-security; urgency=medium
 
   * SECURITY REGRESSION: regression when parsing icap and ecap protocols
diff --git a/debian/squid.resolvconf b/debian/squid.resolvconf
index d7676d0..2622ea5 100644
--- a/debian/squid.resolvconf
+++ b/debian/squid.resolvconf
@@ -5,5 +5,10 @@ PATH="/usr/sbin:/usr/bin:/sbin:/bin"
 # Make squid aware of changes to resolv.conf
 # Avoid reload before /usr is mounted
 if [ -d /usr/sbin ] ; then
-   invoke-rc.d squid reload || true
+   if [ -d /run/systemd ]; then
+      systemctl is-active -q squid.service && \
+         systemctl reload --no-block squid.service > /dev/null
+   else
+      invoke-rc.d squid reload || true
+   fi
 fi