diff --git a/CHANGELOG.md b/CHANGELOG.md index adc6ff3..c409f3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ SPDX-FileCopyrightText: 2024 Foundation Devices Inc. SPDX-License-Identifier: MIT --> +## 0.2.1 + +* Tear down the Tor client deterministically on `stop()`: await the proxy + accept loop, cancel accepted connections, and dispose the Rust client + instead of waiting for Dart GC, which left `tor_cache/dir.lock` held and + forced a restarted client's directory store into silent read-only mode. + ## 0.0.9 * Bumped arti to version 1.4.3 diff --git a/example/pubspec.lock b/example/pubspec.lock index 0b6256a..220c4eb 100644 --- a/example/pubspec.lock +++ b/example/pubspec.lock @@ -398,7 +398,7 @@ packages: path: ".." relative: true source: path - version: "0.2.0" + version: "0.2.1" typed_data: dependency: transitive description: diff --git a/ios/tor/rust_lib_tor.xcframework/Info.plist b/ios/tor/rust_lib_tor.xcframework/Info.plist index edf8576..daadedb 100644 --- a/ios/tor/rust_lib_tor.xcframework/Info.plist +++ b/ios/tor/rust_lib_tor.xcframework/Info.plist @@ -8,32 +8,32 @@ BinaryPath rust_lib_tor.framework/rust_lib_tor LibraryIdentifier - ios-arm64 + ios-arm64_x86_64-simulator LibraryPath rust_lib_tor.framework SupportedArchitectures arm64 + x86_64 SupportedPlatform ios + SupportedPlatformVariant + simulator BinaryPath rust_lib_tor.framework/rust_lib_tor LibraryIdentifier - ios-arm64_x86_64-simulator + ios-arm64 LibraryPath rust_lib_tor.framework SupportedArchitectures arm64 - x86_64 SupportedPlatform ios - SupportedPlatformVariant - simulator CFBundlePackageType diff --git a/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/Info.plist b/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/Info.plist index bbd88fd..907840d 100644 --- a/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/Info.plist +++ b/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType FMWK CFBundleShortVersionString - 0.2.0 + 0.2.1 CFBundleSupportedPlatforms iPhoneOS diff --git a/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/rust_lib_tor b/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/rust_lib_tor index bce53ac..9a1e02f 100755 Binary files a/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/rust_lib_tor and b/ios/tor/rust_lib_tor.xcframework/ios-arm64/rust_lib_tor.framework/rust_lib_tor differ diff --git a/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/Info.plist b/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/Info.plist index 56f3d93..2ee1570 100644 --- a/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/Info.plist +++ b/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/Info.plist @@ -15,7 +15,7 @@ CFBundlePackageType FMWK CFBundleShortVersionString - 0.2.0 + 0.2.1 CFBundleSupportedPlatforms iPhoneSimulator diff --git a/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/rust_lib_tor b/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/rust_lib_tor index 875ed1a..378a0c4 100755 Binary files a/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/rust_lib_tor and b/ios/tor/rust_lib_tor.xcframework/ios-arm64_x86_64-simulator/rust_lib_tor.framework/rust_lib_tor differ diff --git a/lib/tor.dart b/lib/tor.dart index 1356e2e..b39b6d3 100644 --- a/lib/tor.dart +++ b/lib/tor.dart @@ -48,8 +48,7 @@ class Tor { bool _started = false; /// True while the client is starting or re-bootstrapping. - bool get starting => - _startInFlight != null || _bootstrapInFlight != null; + bool get starting => _startInFlight != null || _bootstrapInFlight != null; Future? _startInFlight; Future? _bootstrapInFlight; @@ -197,6 +196,9 @@ class Tor { _client = torInstance.client; _proxy = torInstance.proxy; _proxyPort = torInstance.socksPort; + // The getters above clone; free the container now instead of at GC so + // it cannot keep an extra client reference (and dir.lock) alive. + torInstance.dispose(); _started = true; _bootstrapped = true; // startTor creates a bootstrapped client @@ -258,6 +260,7 @@ class Tor { /// Stops the proxy Future stop() async { final proxy = _proxy; + final client = _client; // Stop publishing the route before awaiting native shutdown so callers // cannot start new work against a proxy that is being torn down. @@ -268,22 +271,14 @@ class Tor { _bootstrapped = false; broadcastState(); - if (proxy == null) { - return; - } - try { - // This is now safe! FRB catches any panic and throws PanicException - await rust.stopProxy(proxy: proxy); - } on rust.TorError catch (e) { - if (kDebugMode) { - print('Error stopping proxy: $e'); - } - } on PanicException catch (e) { - // Previously this would SIGABRT the app, now it's catchable! - if (kDebugMode) { - print('Proxy stop panicked (caught safely): ${e.message}'); + if (proxy != null) { + await rust.stopProxy(proxy: proxy); } + } finally { + // Drop the Rust client now instead of at GC: a lingering client holds + // tor_cache/dir.lock, forcing a restarted client's dirmgr into read-only. + client?.dispose(); } } diff --git a/pubspec.yaml b/pubspec.yaml index 46d2821..1466b51 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -4,7 +4,7 @@ name: tor description: A multi-platform Flutter plugin for managing a Tor proxy. Based on arti. -version: 0.2.0 +version: 0.2.1 homepage: https://github.com/Foundation-Devices/tor environment: diff --git a/rust/Cargo.lock b/rust/Cargo.lock index aeb65ec..074088f 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -3326,21 +3326,24 @@ dependencies = [ [[package]] name = "rust_lib_tor" -version = "0.2.0" +version = "0.2.1" dependencies = [ "anyhow", "arti", "arti-client", "flutter_rust_bridge", + "futures", "lazy_static", "libc", "liblzma", "log", "rlimit 0.10.2", "security-framework 2.10.0", + "tempfile", "thiserror 1.0.69", "time", "tokio", + "tokio-util", "tor-config", "tor-rtcompat", ] @@ -4188,6 +4191,7 @@ dependencies = [ "futures-core", "futures-io", "futures-sink", + "futures-util", "pin-project-lite", "tokio", ] diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 617bc19..1685893 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -4,7 +4,7 @@ [package] name = "rust_lib_tor" -version = "0.2.0" +version = "0.2.1" authors = ["Igor Cota "] edition = "2021" @@ -13,8 +13,10 @@ crate-type = ["cdylib", "staticlib", "rlib"] [dependencies] flutter_rust_bridge = "=2.11.1" +futures = "0.3" lazy_static = "1.4" tokio = { version = "1", features = ["full"] } +tokio-util = { version = "0.7", features = ["rt"] } libc = "0.2" liblzma = { version = "0.4.7", features = ["static"] } arti-client = { version = "0.44.0", features = ["static", "onion-service-client"] } @@ -27,6 +29,9 @@ anyhow = "1.0.79" time = "0.3.36" thiserror = "1.0" +[dev-dependencies] +tempfile = "3" + [target.'cfg(target_os = "ios")'.dependencies] # Specific version for iOS security-framework = "=2.10.0" diff --git a/rust/src/api/tor.rs b/rust/src/api/tor.rs index eef5672..693993c 100644 --- a/rust/src/api/tor.rs +++ b/rust/src/api/tor.rs @@ -7,17 +7,146 @@ use arti::proxy::{self, ListenProtocols}; use arti_client::config::CfgPath; use arti_client::{DormantMode, TorClient, TorClientConfig}; use flutter_rust_bridge::frb; +use futures::future::FutureObj; +use futures::task::{Spawn, SpawnError}; use lazy_static::lazy_static; +use std::future::Future; use std::io; use std::sync::Arc; -use tokio::runtime::{Builder, Runtime}; +use tokio::runtime::{Builder, Runtime as TokioRuntime}; use tokio::task::JoinHandle; +use tokio_util::sync::CancellationToken; +use tokio_util::task::TaskTracker; use tor_config::Listen; use tor_rtcompat::tokio::TokioNativeTlsRuntime; -use tor_rtcompat::{NetStreamProvider, TcpListenOptions, ToplevelBlockOn}; +use tor_rtcompat::{ + Blocking, CompoundRuntime, NetStreamProvider, TcpListenOptions, ToplevelBlockOn, +}; lazy_static! { - static ref RUNTIME: io::Result = Builder::new_multi_thread().enable_all().build(); + static ref RUNTIME: io::Result = Builder::new_multi_thread().enable_all().build(); +} + +const PROXY_SHUTDOWN_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); + +type TrackedTorRuntime = CompoundRuntime< + TrackedSpawner, + TokioNativeTlsRuntime, + TokioNativeTlsRuntime, + TokioNativeTlsRuntime, + TokioNativeTlsRuntime, + TokioNativeTlsRuntime, + TokioNativeTlsRuntime, +>; + +#[derive(Clone, Debug, Default)] +struct TrackedTasks { + tracker: TaskTracker, + cancellation: CancellationToken, +} + +impl TrackedTasks { + fn stop_and_wait(&self) -> Result<(), TorError> { + self.cancellation.cancel(); + self.tracker.close(); + let rt = RUNTIME + .as_ref() + .map_err(|error| TorError::RuntimeError(error.to_string()))?; + if rt + .block_on(async { + tokio::time::timeout(PROXY_SHUTDOWN_TIMEOUT, self.tracker.wait()).await + }) + .is_err() + { + Err(TorError::ProxyStopError( + "Timed out stopping Tor client tasks".to_owned(), + )) + } else { + Ok(()) + } + } +} + +#[derive(Clone, Debug)] +struct TrackedSpawner { + runtime: TokioNativeTlsRuntime, + tasks: TrackedTasks, +} + +impl TrackedSpawner { + fn new(runtime: TokioNativeTlsRuntime) -> Self { + Self { + runtime, + tasks: TrackedTasks::default(), + } + } +} + +impl Spawn for TrackedSpawner { + fn spawn_obj(&self, future: FutureObj<'static, ()>) -> Result<(), SpawnError> { + if self.tasks.tracker.is_closed() { + return Err(SpawnError::shutdown()); + } + + let cancellation = self.tasks.cancellation.clone(); + let future = self.tasks.tracker.track_future(async move { + tokio::select! { + _ = cancellation.cancelled() => {} + _ = future => {} + } + }); + self.runtime.spawn_obj(Box::new(future).into()) + } +} + +impl Blocking for TrackedSpawner { + type ThreadHandle = ::ThreadHandle; + + fn spawn_blocking(&self, f: F) -> Self::ThreadHandle + where + F: FnOnce() -> T + Send + 'static, + T: Send + 'static, + { + self.runtime.spawn_blocking(f) + } + + fn reenter_block_on(&self, future: F) -> F::Output + where + F: Future, + F::Output: Send + 'static, + { + self.runtime.reenter_block_on(future) + } + + fn blocking_io(&self, f: F) -> impl Future + where + F: FnOnce() -> T + Send + 'static, + T: Send + 'static, + { + self.runtime.blocking_io(f) + } +} + +impl ToplevelBlockOn for TrackedSpawner { + fn block_on(&self, future: F) -> F::Output { + self.runtime.block_on(future) + } +} + +fn create_arti_runtime() -> io::Result<(TrackedTorRuntime, TrackedTasks)> { + let runtime = TokioNativeTlsRuntime::create()?; + let spawner = TrackedSpawner::new(runtime.clone()); + let tasks = spawner.tasks.clone(); + let runtime = CompoundRuntime::new( + spawner, + runtime.clone(), + runtime.clone(), + runtime.clone(), + runtime.clone(), + runtime.clone(), + runtime, + ); + Ok((runtime, tasks)) } /// Custom error types for Tor operations @@ -45,8 +174,8 @@ pub enum TorError { /// Opaque wrapper for TorClient - FRB handles this automatically #[frb(opaque)] pub struct TorClientWrapper { - client: Arc>, - runtime: TokioNativeTlsRuntime, + client: Arc>, + runtime: TrackedTorRuntime, } impl Clone for TorClientWrapper { @@ -61,13 +190,18 @@ impl Clone for TorClientWrapper { /// Opaque wrapper for proxy handle #[frb(opaque)] pub struct TorProxyHandle { - handle: Arc>>>>, + state: Arc>, +} + +struct TorProxyState { + accept_loop: Option>>, + tasks: Option, } impl Clone for TorProxyHandle { fn clone(&self) -> Self { TorProxyHandle { - handle: Arc::clone(&self.handle), + state: Arc::clone(&self.state), } } } @@ -94,8 +228,8 @@ pub fn start_tor( state_dir: String, cache_dir: String, ) -> Result { - let runtime = - TokioNativeTlsRuntime::create().map_err(|e| TorError::RuntimeError(e.to_string()))?; + let (runtime, tasks) = + create_arti_runtime().map_err(|e| TorError::RuntimeError(e.to_string()))?; let mut cfg_builder = TorClientConfig::builder(); cfg_builder @@ -128,7 +262,10 @@ pub fn start_tor( Ok(TorInstance { client: TorClientWrapper { client, runtime }, proxy: TorProxyHandle { - handle: Arc::new(std::sync::Mutex::new(Some(proxy_handle))), + state: Arc::new(std::sync::Mutex::new(TorProxyState { + accept_loop: Some(proxy_handle), + tasks: Some(tasks), + })), }, socks_port, }) @@ -136,7 +273,7 @@ pub fn start_tor( fn start_proxy_internal( port: u16, - client: Arc>, + client: Arc>, ) -> Result>, TorError> { let rt = RUNTIME .as_ref() @@ -199,7 +336,7 @@ pub fn bootstrap(client: &TorClientWrapper) -> Result<(), TorError> { /// Set the client dormant mode /// /// * `soft_mode` - If true, uses Soft dormant mode (keeps some circuits warm) -/// If false, uses Normal mode (full operation) +/// If false, uses Normal mode (full operation) pub fn set_dormant(client: &TorClientWrapper, soft_mode: bool) { let dormant_mode = if soft_mode { DormantMode::Soft @@ -211,22 +348,48 @@ pub fn set_dormant(client: &TorClientWrapper, soft_mode: bool) { /// Stop the Tor proxy /// -/// This safely aborts the proxy task. Previously this could panic -/// and crash the app - with FRB, any panic becomes a catchable exception. +/// Stops the accept loop and the client runtime that owns accepted connections. pub fn stop_proxy(proxy: TorProxyHandle) -> Result<(), TorError> { - // Take the handle out of the Option to abort it - // This ensures we only abort once even if called multiple times - let mut guard = proxy - .handle - .lock() - .map_err(|e| TorError::ProxyStopError(e.to_string()))?; - - if let Some(handle) = guard.take() { - // The abort() call is safe with FRB - any panic becomes PanicException - handle.abort(); - } + let (accept_loop, tasks) = { + let mut state = proxy + .state + .lock() + .map_err(|e| TorError::ProxyStopError(e.to_string()))?; + (state.accept_loop.take(), state.tasks.take()) + }; + + let accept_result = match accept_loop { + Some(handle) => { + handle.abort(); + match RUNTIME.as_ref() { + Ok(rt) => rt.block_on(async { + // A loop that already ended - by its own error or a panic - + // has released its listeners and client clone, so only an + // unfinished task leaves teardown unconfirmed. + match tokio::time::timeout(PROXY_SHUTDOWN_TIMEOUT, handle).await { + Err(_) => Err(TorError::ProxyStopError( + "Timed out stopping Tor proxy accept loop".to_owned(), + )), + Ok(Ok(Err(error))) => { + log::warn!("Tor proxy accept loop had already failed: {error}"); + Ok(()) + } + Ok(Err(error)) if !error.is_cancelled() => { + log::warn!("Tor proxy accept loop panicked: {error}"); + Ok(()) + } + Ok(_) => Ok(()), + } + }), + Err(error) => Err(TorError::RuntimeError(error.to_string())), + } + } + None => Ok(()), + }; + + let task_result = tasks.map_or(Ok(()), |tasks| tasks.stop_and_wait()); - Ok(()) + accept_result.and(task_result) } /// Test function to verify library linking @@ -256,3 +419,92 @@ pub fn get_nofile_limit() -> Result { pub fn set_nofile_limit(_limit: u64) -> Result { Ok(0) // Not applicable on Windows } + +#[cfg(test)] +mod tests { + use super::*; + use arti_client::config::TorClientConfigBuilder; + use std::net::TcpStream; + use std::thread; + use std::time::{Duration, Instant}; + use tor_rtcompat::NetStreamListener; + + #[test] + fn stop_proxy_drops_an_idle_connection() { + let state_dir = tempfile::tempdir().unwrap(); + let cache_dir = tempfile::tempdir().unwrap(); + let config = TorClientConfigBuilder::from_directories(state_dir.path(), cache_dir.path()) + .build() + .unwrap(); + let (runtime, tasks) = create_arti_runtime().unwrap(); + let client = TorClient::with_runtime(runtime.clone()) + .config(config) + .create_unbootstrapped() + .unwrap(); + let baseline_client_references = Arc::strong_count(&client); + let listen_address: std::net::SocketAddr = "127.0.0.1:0".parse().unwrap(); + let (listener, address) = runtime.block_on(async { + let listener = runtime + .listen(&listen_address, &TcpListenOptions::default()) + .await + .unwrap(); + let address = listener.local_addr().unwrap(); + (listener, address) + }); + let proxy_task = RUNTIME + .as_ref() + .unwrap() + .spawn(proxy::run_proxy_with_listeners( + Arc::clone(&client), + vec![listener], + ListenProtocols::SocksOnly, + None, + )); + let _idle_connection = TcpStream::connect(address).unwrap(); + + let deadline = Instant::now() + Duration::from_secs(1); + while Arc::strong_count(&client) <= baseline_client_references + 1 + && Instant::now() < deadline + { + thread::sleep(Duration::from_millis(10)); + } + assert!( + Arc::strong_count(&client) > baseline_client_references + 1, + "the idle connection was not accepted" + ); + + stop_proxy(TorProxyHandle { + state: Arc::new(std::sync::Mutex::new(TorProxyState { + accept_loop: Some(proxy_task), + tasks: Some(tasks), + })), + }) + .unwrap(); + + assert_eq!(Arc::strong_count(&client), baseline_client_references); + } + + #[test] + fn stop_proxy_succeeds_when_the_accept_loop_already_failed() { + let accept_loop = RUNTIME + .as_ref() + .unwrap() + .spawn(async { Err(anyhow::anyhow!("fatal accept error")) }); + + let deadline = Instant::now() + Duration::from_secs(1); + while !accept_loop.is_finished() && Instant::now() < deadline { + thread::sleep(Duration::from_millis(10)); + } + assert!(accept_loop.is_finished(), "the accept loop did not finish"); + + // A proxy that already died is exactly when the caller has to be able + // to restart, so a stale accept-loop error must not fail teardown. + stop_proxy(TorProxyHandle { + state: Arc::new(std::sync::Mutex::new(TorProxyState { + accept_loop: Some(accept_loop), + tasks: None, + })), + }) + .unwrap(); + } +} diff --git a/scripts/build_apple_xcframework.sh b/scripts/build_apple_xcframework.sh index 16f86e5..2de36d3 100755 --- a/scripts/build_apple_xcframework.sh +++ b/scripts/build_apple_xcframework.sh @@ -181,7 +181,7 @@ EOF CFBundlePackageType FMWK CFBundleShortVersionString - 0.2.0 + $CRATE_VERSION CFBundleSupportedPlatforms $platform_name @@ -237,6 +237,13 @@ require_tool lipo require_tool install_name_tool require_tool xcodebuild +CRATE_PACKAGE_ID="$(cargo pkgid --manifest-path "$CRATE_DIR/Cargo.toml")" +CRATE_VERSION="${CRATE_PACKAGE_ID##*@}" +if [ "$CRATE_VERSION" = "$CRATE_PACKAGE_ID" ]; then + echo "Could not determine $CRATE_NAME version from Cargo metadata" >&2 + exit 70 +fi + mkdir -p "$BUILD_DIR/merged" "$BUILD_DIR/frameworks" APPLE_TARGETS=(