From cdd3e4d25c4429201eb25622f975b6a6b5dec359 Mon Sep 17 00:00:00 2001
From: Thomas Eizinger <thomas@eizinger.io>
Date: Fri, 8 Nov 2024 05:01:37 +0000
Subject: [PATCH] fix(headless-client): don't fuse futures outside of the loop
 (#7287)

When waiting on multiple futures concurrently within a loop, it is
important that they all get re-created whenever one of them resolves.
Currently, due to the `.fuse` call, the SIGHUP signal can only be sent
once and future signals get ignored.

As a more general fix, I swapped the `futures::select!` macro to the
`tokio::select!` macro which allows referencing these futures without
pinning and fusing. Ideally, we'd not use any of these macros here and
write our own eventloop but that is a larger refactoring.
---
 rust/headless-client/src/main.rs              | 18 ++++++------------
 website/src/components/Changelog/Headless.tsx |  4 ++++
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/rust/headless-client/src/main.rs b/rust/headless-client/src/main.rs
index aa2042918..92c9db157 100644
--- a/rust/headless-client/src/main.rs
+++ b/rust/headless-client/src/main.rs
@@ -14,7 +14,7 @@ use firezone_headless_client::{
 };
 use firezone_logging::{anyhow_dyn_err, telemetry_span};
 use firezone_telemetry::Telemetry;
-use futures::{FutureExt as _, StreamExt as _};
+use futures::StreamExt as _;
 use phoenix_channel::get_user_agent;
 use phoenix_channel::LoginUrl;
 use phoenix_channel::PhoenixChannel;
@@ -22,7 +22,6 @@ use secrecy::{Secret, SecretString};
 use std::{
     collections::BTreeMap,
     path::{Path, PathBuf},
-    pin::pin,
     sync::Arc,
 };
 use tokio::{sync::mpsc, time::Instant};
@@ -233,8 +232,6 @@ fn main() -> Result<()> {
 
         let mut terminate = signals::Terminate::new()?;
         let mut hangup = signals::Hangup::new()?;
-        let mut terminate = pin!(terminate.recv().fuse());
-        let mut hangup = pin!(hangup.recv().fuse());
 
         let mut tun_device = TunDeviceManager::new(ip_packet::PACKET_SIZE)?;
         let mut cb_rx = ReceiverStream::new(cb_rx).fuse();
@@ -258,20 +255,17 @@ fn main() -> Result<()> {
         drop(connect_span);
 
         let result = loop {
-            let mut dns_changed = pin!(dns_notifier.notified().fuse());
-            let mut network_changed = pin!(network_notifier.notified().fuse());
-
-            let cb = futures::select! {
-                () = terminate => {
+            let cb = tokio::select! {
+                () = terminate.recv() => {
                     tracing::info!("Caught SIGINT / SIGTERM / Ctrl+C");
                     break Ok(());
                 },
-                () = hangup => {
+                () = hangup.recv() => {
                     tracing::info!("Caught SIGHUP");
                     session.reset();
                     continue;
                 },
-                result = dns_changed => {
+                result = dns_notifier.notified() => {
                     result?;
                     // If the DNS control method is not `systemd-resolved`
                     // then we'll use polling here, so no point logging every 5 seconds that we're checking the DNS
@@ -279,7 +273,7 @@ fn main() -> Result<()> {
                     session.set_dns(dns_controller.system_resolvers());
                     continue;
                 },
-                result = network_changed => {
+                result = network_notifier.notified() => {
                     result?;
                     tracing::info!("Network change, resetting Session");
                     session.reset();
diff --git a/website/src/components/Changelog/Headless.tsx b/website/src/components/Changelog/Headless.tsx
index 5ee3455ef..46f7aaa3e 100644
--- a/website/src/components/Changelog/Headless.tsx
+++ b/website/src/components/Changelog/Headless.tsx
@@ -18,6 +18,10 @@ export default function Headless() {
         <ChangeItem pull="7265">
           Prevents re-connections to the portal from hanging for longer than 5s.
         </ChangeItem>
+        <ChangeItem pull="7287">
+          Fixes an issue where subsequent SIGHUP signals after the first one were
+          ignored.
+        </ChangeItem>
       </Unreleased>
       <Entry version="1.3.5" date={new Date("2024-10-31")}>
         <ChangeItem>Handles DNS queries over TCP correctly.</ChangeItem>