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.
This commit is contained in:
Thomas Eizinger
2024-11-08 05:01:37 +00:00
committed by GitHub
parent 3a7101bac0
commit cdd3e4d25c
2 changed files with 10 additions and 12 deletions

View File

@@ -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();

View File

@@ -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>