diff --git a/talpid-core/Cargo.toml b/talpid-core/Cargo.toml index 7ec3fd20f288..0cc71a439950 100644 --- a/talpid-core/Cargo.toml +++ b/talpid-core/Cargo.toml @@ -103,3 +103,7 @@ features = [ [build-dependencies] tonic-build = { workspace = true, default-features = false, features = ["transport", "prost"] } + + +[dev-dependencies] +tokio = { workspace = true, features = [ "io-util", "test-util", "time" ] } diff --git a/talpid-core/src/split_tunnel/macos/process.rs b/talpid-core/src/split_tunnel/macos/process.rs index d5d00451449b..f633eddc9021 100644 --- a/talpid-core/src/split_tunnel/macos/process.rs +++ b/talpid-core/src/split_tunnel/macos/process.rs @@ -127,6 +127,7 @@ enum NeedFda { /// Return whether `proc` reports that full-disk access is unavailable based on its output /// If it cannot be determined that access is available, it is assumed to be available async fn parse_logger_status( + // TODO: Remove me proc: impl Future>, stdout: impl AsyncRead + Unpin + Send + 'static, stderr: impl AsyncRead + Unpin + Send + 'static, @@ -162,14 +163,6 @@ async fn parse_logger_status( biased; need_full_disk_access = &mut need_full_disk_access => { need_full_disk_access.unwrap_or(NeedFda::No) } - proc_result = proc => { - if let Ok(_exit_status) = proc_result { - // Process exited - return need_full_disk_access.await.unwrap_or(NeedFda::No); - } - // `Child::wait`` returned an error - NeedFda::No - } // Timed out while checking for full-disk access _ = deadline => NeedFda::No, } @@ -564,12 +557,29 @@ fn check_os_version_support_inner(version: MacosVersion) -> Result<(), Error> { #[cfg(test)] mod test { - use super::{ - check_os_version_support_inner, parse_logger_status, NeedFda, EARLY_FAIL_TIMEOUT, - MIN_OS_VERSION, - }; + use super::*; + use std::{process::ExitStatus, time::Duration}; + use talpid_platform_metadata::MacosVersion; + use tokio::io::{simplex, AsyncWriteExt}; + + /// A mock-version of stdout. [SimplexStream] implements [AsyncRead], so it can be used to test + /// [parse_logger_status]. + fn output(msg: &'static str, lag: Duration) -> impl AsyncRead + Unpin + Send + 'static { + // Ensure that 'msg' contains a newline to prevent user errors + assert!( + msg.contains('\n'), + "Message does not contain a newline!! Make sure to add a newline to '{msg}'" + ); + let (stdout_read, mut stdout_write) = simplex(msg.as_bytes().len()); + // "print" to "stdout" after `duration`. + tokio::spawn(async move { + tokio::time::sleep(lag).await; + stdout_write.write_all(msg.as_bytes()).await.unwrap(); + }); + stdout_read + } #[test] fn test_min_os_version() { @@ -643,4 +653,49 @@ mod test { "expected 'NeedFda::No' on immediate exit", ); } + + /// Check that [parse_logger_status] returns within a reasonable timeframe. + /// "Reasonable" being within [EARLY_FAIL_TIMEOUT]. + #[tokio::test] + async fn test_parse_logger_status_responsive() { + tokio::time::pause(); + let start = tokio::time::Instant::now(); + let stdout = output("This will never be printed\n", Duration::MAX); + let stderr = output( + "ES_NEW_CLIENT_RESULT_ERR_NOT_PERMITTED\n", + EARLY_FAIL_TIMEOUT / 2, + ); + tokio::time::resume(); + + let need_fda = + parse_logger_status(async { Ok(ExitStatus::default()) }, stdout, stderr).await; + + tokio::time::pause(); + + assert_eq!( + need_fda, + NeedFda::Yes, + "expected 'NeedFda::Yes' when ES_NEW_CLIENT_RESULT_ERR_NOT_PERMITTED was eventually printed to stderr" + ); + + // Assert that we did not spend more time waiting than we should + assert!(start.elapsed() < EARLY_FAIL_TIMEOUT); + } + + /// Check that [parse_logger_status] doesn't get stuck because nothing is ever output + /// to stdout. + #[tokio::test] + async fn test_parse_logger_status_hogged() { + let stdout = output("This will never be printed\n", Duration::MAX); + let stderr = output("This will never be printed\n", Duration::MAX); + + let need_fda = + parse_logger_status(async { Ok(ExitStatus::default()) }, stdout, stderr).await; + + assert_eq!( + need_fda, + NeedFda::No, + "expected 'NeedFda::No' when nothing was ever printed to stdout or stderr" + ); + } }