diff --git a/Cargo.lock b/Cargo.lock index aa163b90c..e1100db3c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3825,6 +3825,7 @@ dependencies = [ "bytes", "chrono", "futures-util", + "http 1.4.0", "lazy_static", "libloading 0.8.9", "libwebrtc", diff --git a/livekit-api/src/signal_client/mod.rs b/livekit-api/src/signal_client/mod.rs index c929dde9e..e2448d1d1 100644 --- a/livekit-api/src/signal_client/mod.rs +++ b/livekit-api/src/signal_client/mod.rs @@ -238,15 +238,25 @@ impl SignalClient { // if every region fails the caller sees why the last region // connection failed. let mut last_err = err; - for url in urls.iter() { - log::info!("fallback connection to: {}", url); - match SignalInner::connect(url, token, options.clone(), publisher_offer.clone()) - .await + for region_url in urls.iter() { + log::info!("fallback connection to: {}", region_url); + match SignalInner::connect( + region_url, + token, + options.clone(), + publisher_offer.clone(), + ) + .await { Ok((inner, join_response, stream_events)) => { return Ok(handle_success(inner, join_response, stream_events)) } - Err(region_conn_err) => last_err = region_conn_err, + Err(region_conn_err) => { + // This region is unreachable; drop it from the cache + // so the next attempt doesn't hand it out again. + RegionUrlProvider::mark_failed(url, region_url); + last_err = region_conn_err; + } } } @@ -1299,7 +1309,7 @@ mod tests { let endpoint = format!("http://127.0.0.1:{}/settings/regions", addr.port()); let result = region::fetch_from_endpoint(&endpoint, "fake-token").await; - let urls = result.unwrap(); + let (urls, _max_age) = result.unwrap(); assert_eq!( urls, vec![ diff --git a/livekit-api/src/signal_client/region.rs b/livekit-api/src/signal_client/region.rs index d103e56b7..6938ae7ad 100644 --- a/livekit-api/src/signal_client/region.rs +++ b/livekit-api/src/signal_client/region.rs @@ -12,15 +12,136 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::error::Error as StdError; - -use http::header::{HeaderMap, HeaderValue, AUTHORIZATION}; +use std::{ + collections::HashMap, + error::Error as StdError, + sync::{Arc, OnceLock}, + time::{Duration, Instant}, +}; + +use http::header::{HeaderMap, HeaderValue, AUTHORIZATION, CACHE_CONTROL}; +use parking_lot::Mutex; use serde::Deserialize; +use tokio::sync::Mutex as AsyncMutex; use crate::http_client; use super::{SignalError, SignalResult, REGION_FETCH_TIMEOUT}; +struct CachedRegions { + urls: Vec, + fetched_at: Instant, + /// Effective lifetime of this entry: the server's `Cache-Control: max-age` + /// when present, otherwise [`RegionCache::default_ttl`]. + ttl: Duration, +} + +/// Outcome of a [`RegionCache::get`] lookup. +enum Cached { + /// Entry exists and is within the TTL — safe to use without re-fetching. + Fresh(Vec), + /// Entry exists but is older than the TTL — the caller should re-fetch, but + /// may fall back to these URLs if the re-fetch fails. + Stale(Vec), + /// No entry for this host. + Miss, +} + +/// Process-wide region-list cache keyed by host, mirroring client-sdk-js's +/// static `RegionUrlProvider.cache`. Persisting it here (rather than on a +/// per-connection object) means it survives across reconnect attempts — each of +/// which rebuilds the SignalClient — so the reconnect loop does not re-pay the +/// region fetch on every attempt. +struct RegionCache { + entries: Mutex>, + /// Per-host locks that serialise in-flight fetches, so concurrent cache + /// misses for the same host collapse into a single network request rather + /// than each issuing their own (single-flight). + fetch_locks: Mutex>>>, + default_ttl: Duration, +} + +impl RegionCache { + /// Fallback entry lifetime, used when the server's region response carries + /// no `Cache-Control: max-age`. Matches client-sdk-js's `DEFAULT_MAX_AGE_MS`. + const DEFAULT_TTL: Duration = Duration::from_secs(5); + + fn shared() -> &'static RegionCache { + static CACHE: OnceLock = OnceLock::new(); + CACHE.get_or_init(|| Self::new(Self::DEFAULT_TTL)) + } + + fn new(default_ttl: Duration) -> Self { + Self { + entries: Mutex::new(HashMap::new()), + fetch_locks: Mutex::new(HashMap::new()), + default_ttl, + } + } + + /// Returns the per-host fetch lock for `host`, creating it on first use. + /// Held across the network request so only one fetch per host runs at a + /// time; callers that wait on it then pick up the result from the cache. + fn fetch_lock(&self, host: &str) -> Arc> { + self.fetch_locks + .lock() + .entry(host.to_string()) + .or_insert_with(|| Arc::new(AsyncMutex::new(()))) + .clone() + } + + /// Looks up the cached region URLs for `host`, reporting whether the entry + /// is fresh (within its TTL), stale, or absent. A stale entry is retained so + /// callers can fall back to it when a re-fetch fails. + fn get(&self, host: &str) -> Cached { + let entries = self.entries.lock(); + match entries.get(host) { + Some(e) if e.fetched_at.elapsed() < e.ttl => Cached::Fresh(e.urls.clone()), + Some(e) => Cached::Stale(e.urls.clone()), + None => Cached::Miss, + } + } + + /// Stores `urls` for `host`, honouring the server's `Cache-Control: max-age` + /// (`max_age`) as the entry's TTL and falling back to [`Self::default_ttl`] + /// when the header is absent. + fn insert(&self, host: String, urls: Vec, max_age: Option) { + let ttl = max_age.unwrap_or(self.default_ttl); + self.entries.lock().insert(host, CachedRegions { urls, fetched_at: Instant::now(), ttl }); + } + + /// Removes `failed_url` from the cached list for `host` so it is not handed + /// out again. If that empties the list, the entry is dropped entirely, + /// forcing a re-fetch on the next lookup. + fn mark_failed(&self, host: &str, failed_url: &str) { + let mut entries = self.entries.lock(); + if let Some(entry) = entries.get_mut(host) { + entry.urls.retain(|u| u != failed_url); + if entry.urls.is_empty() { + entries.remove(host); + } + } + } + + /// Drops the cached entry for `host`, forcing a re-fetch on the next lookup. + fn invalidate(&self, host: &str) { + self.entries.lock().remove(host); + } + + /// Drops every cached entry. + fn clear(&self) { + self.entries.lock().clear(); + } +} + +fn region_host(url: &str) -> SignalResult { + let parsed = url::Url::parse(url).map_err(|err| SignalError::UrlParse(err.to_string()))?; + parsed + .host_str() + .map(|h| h.to_string()) + .ok_or_else(|| SignalError::UrlParse("invalid hostname".into())) +} + /// Converts an error into a string that includes the full error chain. /// This is important for debugging TLS errors, where the root cause /// (e.g., "invalid peer certificate: UnknownIssuer") is often buried @@ -53,20 +174,91 @@ pub struct RegionUrlInfo { } impl RegionUrlProvider { + /// Fetch the ordered list of region signalling URLs for a LiveKit Cloud + /// host. Non-cloud (direct / self-hosted) URLs have no regions, so this + /// returns an empty list. Successful results are cached per host for the + /// server's `Cache-Control: max-age` (or [`RegionCache::DEFAULT_TTL`] when + /// absent); failures are never cached. Once an entry goes stale a re-fetch + /// is attempted, but if it fails the stale entry is returned as a fallback + /// rather than surfacing the error. Concurrent calls for the same host are + /// de-duplicated: only one fetch runs at a time and the rest reuse its + /// result. pub async fn fetch_region_urls(url: &str, token: &str) -> SignalResult> { - if is_cloud_url(url)? { - let endpoint = region_endpoint(url)?; - fetch_from_endpoint(&endpoint, token).await - } else { - Ok(vec![]) + if !is_cloud_url(url)? { + return Ok(vec![]); + } + + let host = region_host(url)?; + let cache = RegionCache::shared(); + + // Fast path: a fresh entry needs neither a fetch nor the fetch lock. + let stale = match cache.get(&host) { + Cached::Fresh(urls) => return Ok(urls), + Cached::Stale(urls) => Some(urls), + Cached::Miss => None, + }; + + // Single-flight: serialise concurrent fetches for the same host so they + // collapse into one network request. + let fetch_lock = cache.fetch_lock(&host); + let _guard = fetch_lock.lock().await; + + // Another caller may have refreshed the entry while we waited on the lock. + if let Cached::Fresh(urls) = cache.get(&host) { + return Ok(urls); + } + + let endpoint = region_endpoint(url)?; + match fetch_from_endpoint(&endpoint, token).await { + Ok((urls, max_age)) => { + cache.insert(host, urls.clone(), max_age); + Ok(urls) + } + // The fresh fetch failed; fall back to the stale entry if we have + // one rather than failing outright. + Err(err) => match stale { + Some(urls) => { + log::warn!("region fetch failed ({err}); using stale cached regions for {host}"); + Ok(urls) + } + None => Err(err), + }, } } + + /// Reports that `failed_url` (a region URL previously returned for `url`'s + /// host) could not be connected to, dropping it from the cache so it is not + /// handed out again. When the host's last region URL is dropped the whole + /// entry is invalidated, forcing a fresh fetch on the next attempt. + pub fn mark_failed(url: &str, failed_url: &str) { + if let Ok(host) = region_host(url) { + RegionCache::shared().mark_failed(&host, failed_url); + } + } + + /// Invalidates the cached region list for `url`'s host, forcing a fresh + /// fetch on the next [`Self::fetch_region_urls`] call. + pub fn invalidate(url: &str) { + if let Ok(host) = region_host(url) { + RegionCache::shared().invalidate(&host); + } + } + + /// Clears the entire region cache. Useful when external state that affects + /// geo routing changes (e.g. the device's network connectivity), since that + /// can invalidate every cached host at once. + pub fn clear() { + RegionCache::shared().clear(); + } } +/// Fetches the region list from `endpoint_url`, returning the ordered URLs +/// together with the server's `Cache-Control: max-age` (if any) so the caller +/// can use it as the cache TTL. pub(crate) async fn fetch_from_endpoint( endpoint_url: &str, token: &str, -) -> SignalResult> { +) -> SignalResult<(Vec, Option)> { let fetch_fut = async { let client = http_client::Client::new(); let mut headers = HeaderMap::new(); @@ -81,11 +273,16 @@ pub(crate) async fn fetch_from_endpoint( if !res.status().is_success() { return Err(SignalError::Client(res.status(), res.text().await.unwrap_or_default())); } + + // Read the cache lifetime before `json()` consumes the response. + let max_age = + res.headers().get(CACHE_CONTROL).and_then(|v| v.to_str().ok()).and_then(parse_max_age); + let res = res .json::() .await .map_err(|e| SignalError::RegionError(error_with_chain(&e)))?; - Ok(res.regions.into_iter().map(|i| i.url).collect()) + Ok((res.regions.into_iter().map(|i| i.url).collect(), max_age)) }; livekit_runtime::timeout(REGION_FETCH_TIMEOUT, fetch_fut) @@ -93,6 +290,17 @@ pub(crate) async fn fetch_from_endpoint( .map_err(|_| SignalError::RegionError("region fetch timed out".into()))? } +/// Parses the `max-age` directive (in seconds) out of a `Cache-Control` header +/// value, e.g. `"max-age=300, public"` -> `Some(300s)`. Returns `None` when the +/// directive is absent or unparseable, leaving the caller on the default TTL. +fn parse_max_age(cache_control: &str) -> Option { + cache_control.split(',').find_map(|directive| { + let (name, value) = directive.split_once('=')?; + name.trim().eq_ignore_ascii_case("max-age").then_some(())?; + value.trim().parse::().ok().map(Duration::from_secs) + }) +} + fn is_cloud_url(url: &str) -> SignalResult { let url = url::Url::parse(url).map_err(|err| SignalError::UrlParse(err.to_string()))?; let host = match url.host() { @@ -276,6 +484,126 @@ mod tests { assert!(!is_cloud_url("wss://livekit.cloud.example.com").unwrap()); } + #[test] + fn test_region_host() { + assert_eq!(region_host("wss://myapp.livekit.cloud").unwrap(), "myapp.livekit.cloud"); + assert_eq!(region_host("https://myapp.livekit.cloud/rtc").unwrap(), "myapp.livekit.cloud"); + assert!(region_host("not a url").is_err()); + } + + #[test] + fn region_cache_reports_fresh_stale_and_miss() { + let cache = RegionCache::new(RegionCache::DEFAULT_TTL); + + let host = "cache-fresh.livekit.cloud"; + assert!(matches!(cache.get(host), Cached::Miss), "unknown host is a miss"); + + let urls = vec!["wss://r1.livekit.cloud".to_string(), "wss://r2.livekit.cloud".to_string()]; + cache.insert(host.to_string(), urls.clone(), None); + assert!( + matches!(cache.get(host), Cached::Fresh(u) if u == urls), + "fresh entry is a fresh hit" + ); + + // An entry older than its TTL is reported as stale (retained for fallback). + let stale_host = "cache-stale.livekit.cloud"; + let stale_urls = vec!["wss://old.livekit.cloud".to_string()]; + if let Some(past) = Instant::now().checked_sub(RegionCache::DEFAULT_TTL * 2) { + cache.entries.lock().insert( + stale_host.to_string(), + CachedRegions { + urls: stale_urls.clone(), + fetched_at: past, + ttl: RegionCache::DEFAULT_TTL, + }, + ); + assert!( + matches!(cache.get(stale_host), Cached::Stale(u) if u == stale_urls), + "expired entry is a stale hit" + ); + } + } + + #[test] + fn region_cache_honors_server_max_age() { + // A short max-age expires before the (longer) default TTL would, proving + // the server's Cache-Control wins over the default. + let cache = RegionCache::new(Duration::from_secs(3600)); + let host = "max-age.livekit.cloud"; + let urls = vec!["wss://r1.livekit.cloud".to_string()]; + + cache.insert(host.to_string(), urls.clone(), Some(Duration::ZERO)); + assert!( + matches!(cache.get(host), Cached::Stale(u) if u == urls), + "max-age=0 entry is immediately stale despite the long default TTL" + ); + } + + #[test] + fn region_cache_mark_failed_prunes_then_drops() { + let cache = RegionCache::new(RegionCache::DEFAULT_TTL); + let host = "mark-failed.livekit.cloud"; + let r1 = "wss://r1.livekit.cloud".to_string(); + let r2 = "wss://r2.livekit.cloud".to_string(); + cache.insert(host.to_string(), vec![r1.clone(), r2.clone()], None); + + // Pruning one URL keeps the entry with the survivors. + cache.mark_failed(host, &r1); + assert!( + matches!(cache.get(host), Cached::Fresh(u) if u == vec![r2.clone()]), + "failed URL is pruned, the rest remain" + ); + + // Removing the last URL drops the entry entirely, forcing a re-fetch. + cache.mark_failed(host, &r2); + assert!(matches!(cache.get(host), Cached::Miss), "emptied entry is dropped"); + + // Marking an unknown host is a no-op. + cache.mark_failed("unknown.livekit.cloud", &r1); + } + + #[test] + fn region_cache_invalidate_and_clear() { + let cache = RegionCache::new(RegionCache::DEFAULT_TTL); + let a = "a.livekit.cloud"; + let b = "b.livekit.cloud"; + let urls = vec!["wss://r.livekit.cloud".to_string()]; + cache.insert(a.to_string(), urls.clone(), None); + cache.insert(b.to_string(), urls.clone(), None); + + // invalidate drops only the targeted host. + cache.invalidate(a); + assert!(matches!(cache.get(a), Cached::Miss), "invalidated host is a miss"); + assert!(matches!(cache.get(b), Cached::Fresh(_)), "other host is untouched"); + + // clear drops everything. + cache.clear(); + assert!(matches!(cache.get(b), Cached::Miss), "clear removes all entries"); + } + + #[test] + fn fetch_lock_is_shared_per_host() { + let cache = RegionCache::new(RegionCache::DEFAULT_TTL); + + // Same host hands back the same lock, so concurrent callers contend on a + // single fetch; distinct hosts get independent locks. + let a1 = cache.fetch_lock("a.livekit.cloud"); + let a2 = cache.fetch_lock("a.livekit.cloud"); + let b = cache.fetch_lock("b.livekit.cloud"); + assert!(Arc::ptr_eq(&a1, &a2), "same host shares one fetch lock"); + assert!(!Arc::ptr_eq(&a1, &b), "different hosts get distinct fetch locks"); + } + + #[test] + fn test_parse_max_age() { + assert_eq!(parse_max_age("max-age=300"), Some(Duration::from_secs(300))); + assert_eq!(parse_max_age("public, max-age=300"), Some(Duration::from_secs(300))); + assert_eq!(parse_max_age("MAX-AGE=0, no-cache"), Some(Duration::ZERO)); + assert_eq!(parse_max_age("no-store"), None); + assert_eq!(parse_max_age("max-age=notanumber"), None); + assert_eq!(parse_max_age(""), None); + } + #[test] fn test_region_endpoint() { assert_eq!( diff --git a/livekit/Cargo.toml b/livekit/Cargo.toml index 11b30bdd3..bd21e62ed 100644 --- a/livekit/Cargo.toml +++ b/livekit/Cargo.toml @@ -59,3 +59,4 @@ anyhow = "1.0.99" test-log = "0.2.18" test-case = "3.3" serial_test = "3.0" +http = "1.1" diff --git a/livekit/specs/signalling-reconnection.allium b/livekit/specs/signalling-reconnection.allium index 67414ec86..373b06b75 100644 --- a/livekit/specs/signalling-reconnection.allium +++ b/livekit/specs/signalling-reconnection.allium @@ -441,7 +441,6 @@ rule ServerRequestsDisconnect { rule CloseEngine { when: CloseEngine(engine) requires: engine.status in {connected, reconnecting} - ensures: engine.reconnect_permission = revoked ensures: engine.status = closed ensures: EngineDisconnected(engine, cause: client_initiated) @guidance @@ -449,7 +448,10 @@ rule CloseEngine { -- non-terminal state — including mid-reconnect, cancelling an in-flight -- reconnect. The implementation signals close_notifier, which breaks the -- reconnect loop's backoff/attempt waits immediately rather than waiting - -- them out; permission is revoked so a late stimulus can't restart it. + -- them out; the `closed` state then stops any further reconnection (a + -- late stimulus spawns a loop that bails on the first is-closed check). + -- Permission is NOT revoked here — like exhaustion, close lands `closed` + -- without revoking (RevokedImpliesClosed asserts only revoked => closed). } -- === Engine: starting / escalating the loop ================================= diff --git a/livekit/src/rtc_engine/mod.rs b/livekit/src/rtc_engine/mod.rs index f85897bda..a1a26f4ae 100644 --- a/livekit/src/rtc_engine/mod.rs +++ b/livekit/src/rtc_engine/mod.rs @@ -503,6 +503,14 @@ impl EngineInner { match try_connect().await { Ok(res) => return Ok(res), Err(e) => { + // A validated auth failure (401/403) will not succeed on + // retry with the same token — surface it immediately instead + // of burning the remaining join attempts. Same classification + // as the reconnect loop (see `auth_failure_reason`). + if auth_failure_reason(&e).is_some() { + log::warn!("authentication rejected during connect ({e}); not retrying"); + return Err(e); + } let attempt_i = i + 1; if i < max_retries { log::warn!( @@ -943,6 +951,16 @@ impl EngineInner { "server requested disconnect during restart".into(), )); } + if let Some(reason) = auth_failure_reason(&err) { + log::warn!( + "authentication rejected during restart ({err}); not retrying" + ); + self.running_handle.write().can_reconnect = false; + self.close(reason).await; + return Err(EngineError::Connection( + "authentication failed during reconnect".into(), + )); + } log::error!("restarting connection failed: {}", err); } } @@ -971,6 +989,16 @@ impl EngineInner { "server requested disconnect during resume".into(), )); } + if let Some(reason) = auth_failure_reason(&err) { + log::warn!( + "authentication rejected during resume ({err}); not retrying" + ); + self.running_handle.write().can_reconnect = false; + self.close(reason).await; + return Err(EngineError::Connection( + "authentication failed during reconnect".into(), + )); + } log::error!("resuming connection failed: {}", err); let mut running_handle = self.running_handle.write(); running_handle.full_reconnect = true; @@ -1153,6 +1181,28 @@ fn leave_disconnect_reason(err: &EngineError) -> Option { None } +/// Inspect a reconnect-attempt error for a genuine authentication/authorization +/// failure (HTTP 401/403). Such a failure will not succeed on retry with the +/// same token, so the reconnect loop should bail out immediately rather than +/// burning every attempt (and hammering the server) with credentials it already +/// knows are rejected. +/// +/// We key on `SignalError::Client(401|403)`, which is produced by the server's +/// `rtc/validate` probe (see [`super`]'s `SignalInner::validate`) — an +/// authoritative classification. We deliberately do NOT key on the raw +/// `WsError::Http` upgrade status, because that can be a fabricated 401 masking a +/// transient server error (e.g. a 503 from a saturated node), which IS +/// retryable. A resume that hits a raw 401 simply escalates to a full reconnect, +/// whose connect path runs `validate()` and surfaces the authoritative status. +fn auth_failure_reason(err: &EngineError) -> Option { + if let EngineError::Signal(SignalError::Client(status, _)) = err { + if matches!(status.as_u16(), 401 | 403) { + return Some(DisconnectReason::JoinFailure); + } + } + None +} + #[cfg(test)] mod tests { use super::*; @@ -1200,4 +1250,47 @@ mod tests { ); } } + + #[test] + fn auth_failure_reason_flags_validated_401_and_403() { + // The server's rtc/validate probe surfaces auth failures as Client(4xx). + for status in [401u16, 403] { + let err = EngineError::Signal(SignalError::Client( + http::StatusCode::from_u16(status).unwrap(), + "invalid token".into(), + )); + assert_eq!( + auth_failure_reason(&err), + Some(DisconnectReason::JoinFailure), + "Client({status}) must be treated as a non-retryable auth failure" + ); + } + } + + fn auth_failure_reason_ignores_other_client_and_server_errors() { + let not_auth = [ + // Other client errors are not auth failures. + EngineError::Signal(SignalError::Client(http::StatusCode::NOT_FOUND, "".into())), + EngineError::Signal(SignalError::Client( + http::StatusCode::TOO_MANY_REQUESTS, + "".into(), + )), + // Server errors (e.g. a saturated node) are retryable. + EngineError::Signal(SignalError::Server( + http::StatusCode::SERVICE_UNAVAILABLE, + "".into(), + )), + // Generic connectivity/internal errors are retryable. + EngineError::Connection("network".into()), + EngineError::Internal("bug".into()), + EngineError::Signal(SignalError::SendError), + EngineError::Signal(SignalError::Timeout("waiting".into())), + ]; + for err in ¬_auth { + assert!( + auth_failure_reason(err).is_none(), + "{err:?} must NOT be treated as an auth failure" + ); + } + } } diff --git a/livekit/src/rtc_engine/reconnect_strategy.rs b/livekit/src/rtc_engine/reconnect_strategy.rs index bfb58b0a0..f5daff08e 100644 --- a/livekit/src/rtc_engine/reconnect_strategy.rs +++ b/livekit/src/rtc_engine/reconnect_strategy.rs @@ -70,10 +70,10 @@ mod tests { // Monotonic non-decreasing and never above the cap. let mut prev = Duration::ZERO; for attempt in 1..=RECONNECT_ATTEMPTS { - let nominal = nominal(attempt); - assert!(nominal >= prev, "backoff must not decrease (attempt {attempt})"); - assert!(nominal <= RECONNECT_MAX_DELAY, "backoff must not exceed the cap"); - prev = nominal; + let nominal_duration = nominal(attempt); + assert!(nominal_duration >= prev, "backoff must not decrease (attempt {attempt})"); + assert!(nominal_duration <= RECONNECT_MAX_DELAY, "backoff must not exceed the cap"); + prev = nominal_duration; } // Late attempts are pinned to the cap, and large attempt indices don't @@ -86,12 +86,12 @@ mod tests { fn backoff_delay_stays_within_nominal_jitter_window() { // Full jitter: every sample must land within [0, nominal(attempt)]. for attempt in 1..=RECONNECT_ATTEMPTS { - let nominal = nominal(attempt); + let nominal_duration = nominal(attempt); for _ in 0..1000 { - let delay = delay(attempt); + let delay_duration = delay(attempt); assert!( - delay <= nominal, - "jittered delay {delay:?} exceeded nominal {nominal:?} (attempt {attempt})" + delay_duration <= nominal_duration, + "jittered delay {delay_duration:?} exceeded nominal {nominal_duration:?} (attempt {attempt})" ); } }