diff --git a/Cargo.lock b/Cargo.lock index 1547eb135307..c02b5d23b37f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2529,6 +2529,7 @@ dependencies = [ "oslog", "serde_json", "shadowsocks-service", + "talpid-future", "talpid-tunnel-config-client", "talpid-types", "tokio", diff --git a/ios/MullvadREST/ApiHandlers/MullvadApiRequestFactory.swift b/ios/MullvadREST/ApiHandlers/MullvadApiRequestFactory.swift index 89bf2dd72503..d40039d558fc 100644 --- a/ios/MullvadREST/ApiHandlers/MullvadApiRequestFactory.swift +++ b/ios/MullvadREST/ApiHandlers/MullvadApiRequestFactory.swift @@ -10,7 +10,7 @@ import MullvadRustRuntime import MullvadTypes enum MullvadApiRequest { - case getAddressList + case getAddressList(retryStrategy: REST.RetryStrategy) } struct MullvadApiRequestFactory { @@ -25,8 +25,12 @@ struct MullvadApiRequestFactory { let rawPointer = Unmanaged.passRetained(pointerClass).toOpaque() return switch request { - case .getAddressList: - MullvadApiCancellable(handle: mullvad_api_get_addresses(apiContext.context, rawPointer)) + case let .getAddressList(retryStrategy): + MullvadApiCancellable(handle: mullvad_api_get_addresses( + apiContext.context, + rawPointer, + retryStrategy.toRustStrategy() + )) } } } diff --git a/ios/MullvadREST/ApiHandlers/RESTAPIProxy.swift b/ios/MullvadREST/ApiHandlers/RESTAPIProxy.swift index 4c8e144d6dc7..1dfd01b545ef 100644 --- a/ios/MullvadREST/ApiHandlers/RESTAPIProxy.swift +++ b/ios/MullvadREST/ApiHandlers/RESTAPIProxy.swift @@ -66,7 +66,7 @@ extension REST { retryStrategy: REST.RetryStrategy, completionHandler: @escaping @Sendable ProxyCompletionHandler<[AnyIPEndpoint]> ) -> Cancellable { - let requestHandler = mullvadApiRequestFactory.makeRequest(.getAddressList) + let requestHandler = mullvadApiRequestFactory.makeRequest(.getAddressList(retryStrategy: retryStrategy)) let responseHandler = rustResponseHandler( decoding: [AnyIPEndpoint].self, @@ -76,7 +76,6 @@ extension REST { let networkOperation = MullvadApiNetworkOperation( name: "get-api-addrs", dispatchQueue: dispatchQueue, - retryStrategy: retryStrategy, requestHandler: requestHandler, responseDecoder: responseDecoder, responseHandler: responseHandler, diff --git a/ios/MullvadREST/ApiHandlers/RESTRustNetworkOperation.swift b/ios/MullvadREST/ApiHandlers/RESTRustNetworkOperation.swift index 1bcd21844483..378da0342561 100644 --- a/ios/MullvadREST/ApiHandlers/RESTRustNetworkOperation.swift +++ b/ios/MullvadREST/ApiHandlers/RESTRustNetworkOperation.swift @@ -21,22 +21,14 @@ extension REST { private let responseHandler: any RESTRustResponseHandler private var networkTask: MullvadApiCancellable? - private let retryStrategy: RetryStrategy - private var retryDelayIterator: AnyIterator - private var retryTimer: DispatchSourceTimer? - private var retryCount = 0 - init( name: String, dispatchQueue: DispatchQueue, - retryStrategy: RetryStrategy, requestHandler: @escaping MullvadApiRequestHandler, responseDecoder: JSONDecoder, responseHandler: some RESTRustResponseHandler, completionHandler: CompletionHandler? = nil ) { - self.retryStrategy = retryStrategy - retryDelayIterator = retryStrategy.makeDelayIterator() self.responseDecoder = responseDecoder self.requestHandler = requestHandler self.responseHandler = responseHandler @@ -53,10 +45,7 @@ extension REST { } override public func operationDidCancel() { - retryTimer?.cancel() networkTask?.cancel() - - retryTimer = nil networkTask = nil } @@ -76,12 +65,7 @@ extension REST { guard let self else { return } if let error = response.restError() { - if response.shouldRetry { - retryRequest(with: error) - } else { - finish(result: .failure(error)) - } - + finish(result: .failure(error)) return } @@ -97,51 +81,6 @@ extension REST { } } } - - private func retryRequest(with error: REST.Error) { - // Check if retry count is not exceeded. - guard retryCount < retryStrategy.maxRetryCount else { - if retryStrategy.maxRetryCount > 0 { - logger.debug("Ran out of retry attempts (\(retryStrategy.maxRetryCount))") - } - finish(result: .failure(error)) - return - } - - // Increment retry count. - retryCount += 1 - - // Retry immediately if retry delay is set to never. - guard retryStrategy.delay != .never else { - startRequest() - return - } - - guard let waitDelay = retryDelayIterator.next() else { - logger.debug("Retry delay iterator failed to produce next value.") - - finish(result: .failure(error)) - return - } - - logger.debug("Retry in \(waitDelay.logFormat()).") - - // Create timer to delay retry. - let timer = DispatchSource.makeTimerSource(queue: dispatchQueue) - - timer.setEventHandler { [weak self] in - self?.startRequest() - } - - timer.setCancelHandler { [weak self] in - self?.finish(result: .failure(OperationError.cancelled)) - } - - timer.schedule(wallDeadline: .now() + waitDelay.timeInterval) - timer.activate() - - retryTimer = timer - } } } diff --git a/ios/MullvadREST/RetryStrategy/RetryStrategy.swift b/ios/MullvadREST/RetryStrategy/RetryStrategy.swift index 3e996a9205c1..4f0d7016af25 100644 --- a/ios/MullvadREST/RetryStrategy/RetryStrategy.swift +++ b/ios/MullvadREST/RetryStrategy/RetryStrategy.swift @@ -7,6 +7,7 @@ // import Foundation +import MullvadRustRuntime import MullvadTypes extension REST { @@ -21,6 +22,23 @@ extension REST { self.applyJitter = applyJitter } + /// The return value of this function *must* be passed to a Rust FFI function that will consume it, otherwise it will leak. + public func toRustStrategy() -> SwiftRetryStrategy { + switch delay { + case .never: + return mullvad_api_retry_strategy_never() + case let .constant(duration): + return mullvad_api_retry_strategy_constant(UInt(maxRetryCount), UInt64(duration.seconds)) + case let .exponentialBackoff(initial, multiplier, maxDelay): + return mullvad_api_retry_strategy_exponential( + UInt(maxRetryCount), + UInt64(initial.seconds), + UInt32(multiplier), + UInt64(maxDelay.seconds) + ) + } + } + public func makeDelayIterator() -> AnyIterator { let inner = delay.makeIterator() diff --git a/ios/MullvadRustRuntime/MullvadApiResponse.swift b/ios/MullvadRustRuntime/MullvadApiResponse.swift index a709342b4e19..7836d43971fe 100644 --- a/ios/MullvadRustRuntime/MullvadApiResponse.swift +++ b/ios/MullvadRustRuntime/MullvadApiResponse.swift @@ -45,10 +45,6 @@ public class MullvadApiResponse { } } - public var shouldRetry: Bool { - response.should_retry - } - public var success: Bool { response.success } diff --git a/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h b/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h index faae315d6de0..e054de004482 100644 --- a/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h +++ b/ios/MullvadRustRuntime/include/mullvad_rust_runtime.h @@ -26,6 +26,8 @@ typedef struct ExchangeCancelToken ExchangeCancelToken; typedef struct RequestCancelHandle RequestCancelHandle; +typedef struct RetryStrategy RetryStrategy; + typedef struct SwiftApiContext { const struct ApiContext *_0; } SwiftApiContext; @@ -34,6 +36,10 @@ typedef struct SwiftCancelHandle { struct RequestCancelHandle *ptr; } SwiftCancelHandle; +typedef struct SwiftRetryStrategy { + struct RetryStrategy *_0; +} SwiftRetryStrategy; + typedef struct SwiftMullvadApiResponse { uint8_t *body; uintptr_t body_size; @@ -41,8 +47,6 @@ typedef struct SwiftMullvadApiResponse { uint8_t *error_description; uint8_t *server_response_code; bool success; - bool should_retry; - uint64_t retry_after; } SwiftMullvadApiResponse; typedef struct CompletionCookie { @@ -106,7 +110,8 @@ struct SwiftApiContext mullvad_api_init_new(const uint8_t *host, * This function is not safe to call multiple times with the same `CompletionCookie`. */ struct SwiftCancelHandle mullvad_api_get_addresses(struct SwiftApiContext api_context, - void *completion_cookie); + void *completion_cookie, + struct SwiftRetryStrategy retry_strategy); /** * Called by the Swift side to signal that a Mullvad API call should be cancelled. @@ -156,6 +161,29 @@ extern void mullvad_api_completion_finish(struct SwiftMullvadApiResponse respons */ void mullvad_response_drop(struct SwiftMullvadApiResponse response); +/** + * Creates a retry strategy that never retries after failure. + * The result needs to be consumed. + */ +struct SwiftRetryStrategy mullvad_api_retry_strategy_never(void); + +/** + * Creates a retry strategy that retries `max_retries` times with a constant delay of `delay_sec`. + * The result needs to be consumed. + */ +struct SwiftRetryStrategy mullvad_api_retry_strategy_constant(uintptr_t max_retries, + uint64_t delay_sec); + +/** + * Creates a retry strategy that retries `max_retries` times with a exponantially increating delay. + * The delay will never exceed `max_delay_sec` + * The result needs to be consumed. + */ +struct SwiftRetryStrategy mullvad_api_retry_strategy_exponential(uintptr_t max_retries, + uint64_t initial_sec, + uint32_t factor, + uint64_t max_delay_sec); + /** * Initializes a valid pointer to an instance of `EncryptedDnsProxyState`. * diff --git a/ios/MullvadTypes/Duration+Extensions.swift b/ios/MullvadTypes/Duration+Extensions.swift index e4a63a6b4abf..e01fa4f883e2 100644 --- a/ios/MullvadTypes/Duration+Extensions.swift +++ b/ios/MullvadTypes/Duration+Extensions.swift @@ -14,6 +14,10 @@ extension Duration { return timeInterval.isFinite } + public var seconds: Int64 { + return components.seconds + } + public var timeInterval: TimeInterval { return TimeInterval(components.seconds) + (TimeInterval(components.attoseconds) * 1e-18) } diff --git a/mullvad-ios/Cargo.toml b/mullvad-ios/Cargo.toml index d1dfea5ba15a..8f95f148d90b 100644 --- a/mullvad-ios/Cargo.toml +++ b/mullvad-ios/Cargo.toml @@ -24,6 +24,7 @@ hyper-util = { workspace = true } tower = { workspace = true } tunnel-obfuscation = { path = "../tunnel-obfuscation" } oslog = "0.2" +talpid-future = { path = "../talpid-future" } talpid-types = { path = "../talpid-types" } talpid-tunnel-config-client = { path = "../talpid-tunnel-config-client" } mullvad-encrypted-dns-proxy = { path = "../mullvad-encrypted-dns-proxy" } diff --git a/mullvad-ios/src/api_client/api.rs b/mullvad-ios/src/api_client/api.rs index ad3069a20b89..847e81f0eb31 100644 --- a/mullvad-ios/src/api_client/api.rs +++ b/mullvad-ios/src/api_client/api.rs @@ -2,11 +2,13 @@ use mullvad_api::{ rest::{self, MullvadRestHandle}, ApiProxy, }; +use talpid_future::retry::retry_future; use super::{ cancellation::{RequestCancelHandle, SwiftCancelHandle}, completion::{CompletionCookie, SwiftCompletionHandler}, response::SwiftMullvadApiResponse, + retry_strategy::{RetryStrategy, SwiftRetryStrategy}, SwiftApiContext, }; @@ -24,6 +26,7 @@ use super::{ pub unsafe extern "C" fn mullvad_api_get_addresses( api_context: SwiftApiContext, completion_cookie: *mut libc::c_void, + retry_strategy: SwiftRetryStrategy, ) -> SwiftCancelHandle { let completion_handler = SwiftCompletionHandler::new(CompletionCookie(completion_cookie)); @@ -33,10 +36,11 @@ pub unsafe extern "C" fn mullvad_api_get_addresses( }; let api_context = api_context.into_rust_context(); + let retry_strategy = unsafe { retry_strategy.into_rust() }; let completion = completion_handler.clone(); let task = tokio_handle.clone().spawn(async move { - match mullvad_api_get_addresses_inner(api_context.rest_handle()).await { + match mullvad_api_get_addresses_inner(api_context.rest_handle(), retry_strategy).await { Ok(response) => completion.finish(response), Err(err) => { log::error!("{err:?}"); @@ -50,9 +54,18 @@ pub unsafe extern "C" fn mullvad_api_get_addresses( async fn mullvad_api_get_addresses_inner( rest_client: MullvadRestHandle, + retry_strategy: RetryStrategy, ) -> Result { let api = ApiProxy::new(rest_client); - let response = api.get_api_addrs_response().await?; + + let future_factory = || api.get_api_addrs_response(); + + let should_retry = |result: &Result<_, rest::Error>| match result { + Err(err) => err.is_network_error(), + Ok(_) => false, + }; + + let response = retry_future(future_factory, should_retry, retry_strategy.delays()).await?; SwiftMullvadApiResponse::with_body(response).await } diff --git a/mullvad-ios/src/api_client/mod.rs b/mullvad-ios/src/api_client/mod.rs index fdda5b0dbbc5..98443fd0d9aa 100644 --- a/mullvad-ios/src/api_client/mod.rs +++ b/mullvad-ios/src/api_client/mod.rs @@ -10,6 +10,7 @@ mod api; mod cancellation; mod completion; mod response; +mod retry_strategy; #[repr(C)] pub struct SwiftApiContext(*const ApiContext); diff --git a/mullvad-ios/src/api_client/response.rs b/mullvad-ios/src/api_client/response.rs index 249f1040bd03..6ffbadb5d6d8 100644 --- a/mullvad-ios/src/api_client/response.rs +++ b/mullvad-ios/src/api_client/response.rs @@ -10,8 +10,6 @@ pub struct SwiftMullvadApiResponse { error_description: *mut u8, server_response_code: *mut u8, success: bool, - should_retry: bool, - retry_after: u64, } impl SwiftMullvadApiResponse { pub async fn with_body(response: Response) -> Result { @@ -28,8 +26,6 @@ impl SwiftMullvadApiResponse { error_description: null_mut(), server_response_code: null_mut(), success: true, - should_retry: false, - retry_after: 0, }) } @@ -44,7 +40,6 @@ impl SwiftMullvadApiResponse { .unwrap_or(null_mut()) }; - let should_retry = err.is_network_error(); let error_description = to_cstr_pointer(err.to_string()); let (status_code, server_response_code): (u16, _) = if let rest::Error::ApiError(status_code, error_code) = err { @@ -60,34 +55,28 @@ impl SwiftMullvadApiResponse { error_description, server_response_code, success: false, - should_retry, - retry_after: 0, } } pub fn cancelled() -> Self { Self { success: false, - should_retry: false, error_description: c"Request was cancelled".to_owned().into_raw().cast(), body: null_mut(), body_size: 0, status_code: 0, server_response_code: null_mut(), - retry_after: 0, } } pub fn no_tokio_runtime() -> Self { Self { success: false, - should_retry: false, error_description: c"Failed to get Tokio runtime".to_owned().into_raw().cast(), body: null_mut(), body_size: 0, status_code: 0, server_response_code: null_mut(), - retry_after: 0, } } } diff --git a/mullvad-ios/src/api_client/retry_strategy.rs b/mullvad-ios/src/api_client/retry_strategy.rs new file mode 100644 index 000000000000..80a3b9668c74 --- /dev/null +++ b/mullvad-ios/src/api_client/retry_strategy.rs @@ -0,0 +1,99 @@ +use std::time::Duration; + +use talpid_future::retry::{ConstantInterval, ExponentialBackoff, Jittered}; + +#[repr(C)] +pub struct SwiftRetryStrategy(*mut RetryStrategy); + +impl SwiftRetryStrategy { + /// # Safety + /// The pointer must be a pointing to a valid instance of a `Box`. + pub unsafe fn into_rust(self) -> RetryStrategy { + *Box::from_raw(self.0) + } +} + +pub struct RetryStrategy { + delays: RetryDelay, + max_retries: usize, +} + +impl RetryStrategy { + pub fn delays(self) -> impl Iterator + Send { + let Self { + delays, + max_retries, + } = self; + + let delays: Box + Send> = match delays { + RetryDelay::Never => Box::new(std::iter::empty()), + RetryDelay::Constant(constant_delays) => Box::new(constant_delays.take(max_retries)), + RetryDelay::Exponential(exponential_delays) => { + Box::new(exponential_delays.take(max_retries)) + } + }; + + Jittered::jitter(delays) + } +} + +#[repr(C)] +pub enum RetryDelay { + Never, + Constant(ConstantInterval), + Exponential(ExponentialBackoff), +} + +/// Creates a retry strategy that never retries after failure. +/// The result needs to be consumed. +#[no_mangle] +pub extern "C" fn mullvad_api_retry_strategy_never() -> SwiftRetryStrategy { + let retry_strategy = RetryStrategy { + delays: RetryDelay::Never, + max_retries: 0, + }; + + let ptr = Box::into_raw(Box::new(retry_strategy)); + SwiftRetryStrategy(ptr) +} + +/// Creates a retry strategy that retries `max_retries` times with a constant delay of `delay_sec`. +/// The result needs to be consumed. +#[no_mangle] +pub extern "C" fn mullvad_api_retry_strategy_constant( + max_retries: usize, + delay_sec: u64, +) -> SwiftRetryStrategy { + let interval = Duration::from_secs(delay_sec); + let retry_strategy = RetryStrategy { + delays: RetryDelay::Constant(ConstantInterval::new(interval, Some(max_retries))), + max_retries: 0, + }; + let ptr = Box::into_raw(Box::new(retry_strategy)); + + SwiftRetryStrategy(ptr) +} + +/// Creates a retry strategy that retries `max_retries` times with a exponantially increating delay. +/// The delay will never exceed `max_delay_sec` +/// The result needs to be consumed. +#[no_mangle] +pub extern "C" fn mullvad_api_retry_strategy_exponential( + max_retries: usize, + initial_sec: u64, + factor: u32, + max_delay_sec: u64, +) -> SwiftRetryStrategy { + let initial_delay = Duration::from_secs(initial_sec); + + let backoff = ExponentialBackoff::new(initial_delay, factor) + .max_delay(Some(Duration::from_secs(max_delay_sec))); + + let retry_strategy = RetryStrategy { + delays: RetryDelay::Exponential(backoff), + max_retries, + }; + + let ptr = Box::into_raw(Box::new(retry_strategy)); + SwiftRetryStrategy(ptr) +}