diff options
| -rw-r--r-- | Cargo.lock | 44 | ||||
| -rw-r--r-- | crates/arti-client/Cargo.toml | 9 | ||||
| -rw-r--r-- | crates/arti-client/README.md | 9 | ||||
| -rw-r--r-- | crates/arti-relay/Cargo.toml | 3 | ||||
| -rw-r--r-- | crates/arti/Cargo.toml | 6 | ||||
| -rw-r--r-- | crates/arti/README.md | 11 | ||||
| -rw-r--r-- | crates/arti/build.rs | 4 | ||||
| -rw-r--r-- | crates/arti/src/arti-example-config.toml | 21 | ||||
| -rw-r--r-- | crates/arti/src/cfg.rs | 48 | ||||
| -rw-r--r-- | crates/tor-config/Cargo.toml | 1 | ||||
| -rw-r--r-- | crates/tor-config/src/misc.rs | 21 | ||||
| -rw-r--r-- | crates/tor-memquota/Cargo.toml | 2 | ||||
| -rw-r--r-- | crates/tor-memquota/semver.md | 4 | ||||
| -rw-r--r-- | crates/tor-memquota/src/config.rs | 324 | ||||
| -rw-r--r-- | crates/tor-memquota/src/internal_prelude.rs | 2 | ||||
| -rw-r--r-- | crates/tor-memquota/src/mtracker.rs | 2 | ||||
| -rwxr-xr-x | maint/downgrade_dependencies | 5 |
17 files changed, 444 insertions, 72 deletions
diff --git a/Cargo.lock b/Cargo.lock index f1b4015bf..5fc912619 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3462,6 +3462,15 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5e0826a989adedc2a244799e823aece04662b66609d96af8dff7ac6df9a8925d" [[package]] +name = "ntapi" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8a3895c6391c39d7fe7ebc444a87eb2991b2a0bc718fdabd071eec617fc68e4" +dependencies = [ + "winapi", +] + +[[package]] name = "nu-ansi-term" version = "0.46.0" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -3597,6 +3606,25 @@ dependencies = [ ] [[package]] +name = "objc2-core-foundation" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1c10c2894a6fed806ade6027bcd50662746363a9589d3ec9d9bef30a4e4bc166" +dependencies = [ + "bitflags 2.9.1", +] + +[[package]] +name = "objc2-io-kit" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "71c1c64d6120e51cd86033f67176b1cb66780c2efe34dec55176f77befd93c0a" +dependencies = [ + "libc", + "objc2-core-foundation", +] + +[[package]] name = "object" version = "0.36.7" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -5358,6 +5386,20 @@ dependencies = [ ] [[package]] +name = "sysinfo" +version = "0.36.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "252800745060e7b9ffb7b2badbd8b31cfa4aa2e61af879d0a3bf2a317c20217d" +dependencies = [ + "libc", + "memchr", + "ntapi", + "objc2-core-foundation", + "objc2-io-kit", + "windows", +] + +[[package]] name = "tap" version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" @@ -6430,6 +6472,7 @@ dependencies = [ name = "tor-memquota" version = "0.32.0" dependencies = [ + "cfg-if", "derive-deftly 1.1.0", "derive_more", "dyn-clone", @@ -6444,6 +6487,7 @@ dependencies = [ "serde_json", "slotmap-careful", "static_assertions", + "sysinfo", "thiserror 2.0.12", "tor-async-utils", "tor-basic-utils", diff --git a/crates/arti-client/Cargo.toml b/crates/arti-client/Cargo.toml index acd9b3e08..744dfe1c1 100644 --- a/crates/arti-client/Cargo.toml +++ b/crates/arti-client/Cargo.toml @@ -19,7 +19,6 @@ default = ["tokio", "native-tls", "compression"] full = [ "anyhow", "keymgr", - "memquota", "onion-service-client", "onion-service-service", "vanguards", @@ -66,7 +65,10 @@ full = [ async-std = ["tor-rtcompat/async-std"] bridge-client = ["tor-guardmgr/bridge-client", "tor-dirmgr/bridge-client"] -memquota = ["tor-memquota/memquota"] +# the "memquota" feature is now a no-op as the functionality is always-enabled, +# but the feature is kept for backwards compatibility +# https://gitlab.torproject.org/tpo/core/arti/-/issues/2030 +memquota = [] tokio = ["tor-rtcompat/tokio", "tor-proto/tokio"] native-tls = ["tor-rtcompat/native-tls"] pt-client = [ @@ -189,8 +191,7 @@ tor-hsservice = { path = "../tor-hsservice", version = "0.32.0", optional = true tor-keymgr = { path = "../tor-keymgr", version = "0.32.0", default-features = false } tor-linkspec = { path = "../tor-linkspec", version = "0.32.0" } tor-llcrypto = { path = "../tor-llcrypto", version = "0.32.0" } -# tor-memquota dependency is unconditional, but most of the code is behind tor-memquota/memquota -tor-memquota = { path = "../tor-memquota", version = "0.32.0", default-features = false } +tor-memquota = { path = "../tor-memquota", version = "0.32.0" } tor-netdir = { path = "../tor-netdir", version = "0.32.0" } tor-netdoc = { path = "../tor-netdoc", version = "0.32.0" } tor-persist = { path = "../tor-persist", version = "0.32.0" } diff --git a/crates/arti-client/README.md b/crates/arti-client/README.md index f8d8792f1..01eb6365f 100644 --- a/crates/arti-client/README.md +++ b/crates/arti-client/README.md @@ -225,7 +225,6 @@ match tor_client.connect(("example.com", 80)).await { * `compression` (default) -- Build support for downloading compressed documents. Requires a C compiler. * `bridge-client` -- Build with support for bridges. -* `memquota` -- Build with support for memory use tracking and limiting. * `onion-service-client` -- Build with support for connecting to onion services. Note that this is not yet as secure as C-Tor and shouldn't be used for security-sensitive purposes. @@ -289,4 +288,12 @@ implementation with another. work reliably. To be explicit: if you want `cargo update` to _only_ make safe changes, then you cannot enable these features. +### Deprecated features + +These features are either not recommended, or are no-op features. +They are included for backwards compatibility. + +* `memquota` -- Memory quota tracking is now always supported, + regardless of if this feature is enabled. + License: MIT OR Apache-2.0 diff --git a/crates/arti-relay/Cargo.toml b/crates/arti-relay/Cargo.toml index c5e796bf9..d946aa6b0 100644 --- a/crates/arti-relay/Cargo.toml +++ b/crates/arti-relay/Cargo.toml @@ -47,8 +47,7 @@ tor-config-path = { path = "../tor-config-path", version = "0.32.0" } tor-error = { path = "../tor-error", version = "0.32.0" } tor-keymgr = { path = "../tor-keymgr", version = "0.32.0", features = ["keymgr", "ephemeral-keystore"] } tor-llcrypto = { path = "../tor-llcrypto", version = "0.32.0" } -# TODO RELAY compile in memquota tracking by default? with a calculated limit maybe, even? -tor-memquota = { version = "0.32.0", path = "../tor-memquota", default-features = false } +tor-memquota = { version = "0.32.0", path = "../tor-memquota" } tor-netdir = { path = "../tor-netdir", version = "0.32.0" } tor-netdoc = { path = "../tor-netdoc", version = "0.32.0" } tor-proto = { path = "../tor-proto", version = "0.32.0", features = ["tokio"] } diff --git a/crates/arti/Cargo.toml b/crates/arti/Cargo.toml index 942962569..cddbbc823 100644 --- a/crates/arti/Cargo.toml +++ b/crates/arti/Cargo.toml @@ -28,7 +28,6 @@ default = [ full = [ "default-runtime", - "memquota", "onion-service-client", "onion-service-service", "vanguards", @@ -74,7 +73,10 @@ dns-proxy = ["hickory-proto"] experimental-api = ["arti-client/experimental-api", "visibility", "__is_experimental"] harden = ["secmem-proc"] keymgr = ["arti-client/keymgr", "__is_experimental"] -memquota = ["arti-client/memquota"] +# the "memquota" feature is now a no-op as the functionality is always-enabled, +# but the feature is kept for backwards compatibility +# https://gitlab.torproject.org/tpo/core/arti/-/issues/2030 +memquota = [] tokio = ["tokio-crate", "arti-client/tokio", "tor-rtcompat/tokio", "tokio-util"] native-tls = ["arti-client/native-tls", "tor-rtcompat/native-tls"] onion-service-client = ["arti-client/onion-service-client"] diff --git a/crates/arti/README.md b/crates/arti/README.md index dfcf8f8c2..5830e41c9 100644 --- a/crates/arti/README.md +++ b/crates/arti/README.md @@ -121,7 +121,6 @@ work. DNS queries over the Tor network. * `harden` (default) -- Build with support for hardening the Arti process by disabling debugger attachment and other local memory-inspection vectors. -* `memquota` -- Build with support for memory use tracking and limiting. * `compression` (default) -- Build support for downloading compressed documents. Requires a C compiler. * `bridge-client` (default) -- Build with support for bridges. @@ -191,6 +190,16 @@ implementation with another. work reliably. To be explicit, if you want `cargo update` to _only_ make correct changes, then you cannot enable these features. +### Deprecated features + +These features are either not recommended, or are no-op features. +They are included for backwards compatibility. +Note that these features will still be enabled if you build with the +`--all-features` cargo flag. + +* `memquota` -- Memory quota tracking is now always supported, + regardless of if this feature is enabled. + [#1795]: https://gitlab.torproject.org/tpo/core/arti/-/issues/1795 ## Limitations diff --git a/crates/arti/build.rs b/crates/arti/build.rs index 00e8d611d..aeb11b5dc 100644 --- a/crates/arti/build.rs +++ b/crates/arti/build.rs @@ -11,5 +11,9 @@ fn main() { // // (We don't want this to be a normal cargo feature because then it would be enabled // with --workspace --all-features, defeating the point.) + // + // NOTE: This was previously used by memquota which was behind a feature flag. Now that we + // always build arti with support for memquota, this is unused. But leaving it in place for now + // anyways in case we want it again in the future. println!(r#"cargo:rustc-check-cfg=cfg(arti_features_precise)"#); } diff --git a/crates/arti/src/arti-example-config.toml b/crates/arti/src/arti-example-config.toml index 4d80738c7..1ce4770ed 100644 --- a/crates/arti/src/arti-example-config.toml +++ b/crates/arti/src/arti-example-config.toml @@ -473,15 +473,26 @@ #max_files = 16384 # Are we limiting memory use and if so to how much? -# The default is unlimited. # -# Maximum memory use, after which reclamation starts: +# Maximum memory use, after which reclamation starts. +# +# The default is "auto", which uses a value derived from the total system +# memory. It should not be assumed that the value used for "auto" will remain +# stable across different versions of arti. The value used for "auto" may also +# take into account other OS-specific features, such as cgroups on Linux. +# +# If `system.memory.low_water` is given as an explicit value, +# `system.memory.max` must also be given as an explicit value. +# # memory.max = "8 GiB" -# (If anything is specified in `[system.memory]`, this value is mandatory.) # -# When reclaiming memory, we stop when we reach this amount: +# When reclaiming memory, we stop when we reach this amount. +# +# The default is "auto", which uses 3/4 of `system.memory.max`. It should not be +# assumed that the value used for "auto" will remain stable across different +# versions of arti. +# # memory.low_water = "6 GiB" -# (The default is 3/4 of `system.memory.max`.) ##### ONION SERVICES # diff --git a/crates/arti/src/cfg.rs b/crates/arti/src/cfg.rs index 5454e7532..b67a4a814 100644 --- a/crates/arti/src/cfg.rs +++ b/crates/arti/src/cfg.rs @@ -1261,37 +1261,23 @@ example config file {which:?}, uncommented={uncommented:?} let result = file.resolve_return_results::<(TorClientConfig, ArtiConfig)>(); - cfg_if::cfg_if! { - if #[cfg(feature = "memquota")] { - let result = result.unwrap(); - - // Test that the example config doesn't have any unrecognised keys - assert_eq!(result.unrecognized, []); - assert_eq!(result.deprecated, []); - - let inner: &tor_memquota::testing::ConfigInner = - result.value.0.system_memory().inner().unwrap(); - - // Test that the example low_water is the default - // value for the example max. - let defaulted_low = tor_memquota::Config::builder() - .max(*inner.max) - .build() - .unwrap(); - let inner_defaulted_low = defaulted_low.inner().unwrap(); - assert_eq!(inner, inner_defaulted_low); - } else if #[cfg(arti_features_precise)] { - // Test that requesting memory quota tracking generates a config error - // if support is compiled out. - let m = result.unwrap_err().report().to_string(); - assert!(m.contains("cargo feature `memquota` disabled"), "{m:?}"); - } else { - // The `tor-memquota/memquota` feature is enabled by default in tor-memquota, - // but the corresponding `memquota` feature is but not enabled here in `arti`. - // so cargo --workspace enables it in a way we can't tell. See arti/build.rs. - println!("not testing memquota config, cannot figure out if it's enabled"); - } - } + let result = result.unwrap(); + + // Test that the example config doesn't have any unrecognised keys + assert_eq!(result.unrecognized, []); + assert_eq!(result.deprecated, []); + + let inner: &tor_memquota::testing::ConfigInner = + result.value.0.system_memory().inner().unwrap(); + + // Test that the example low_water is the default + // value for the example max. + let defaulted_low = tor_memquota::Config::builder() + .max(*inner.max) + .build() + .unwrap(); + let inner_defaulted_low = defaulted_low.inner().unwrap(); + assert_eq!(inner, inner_defaulted_low); } #[test] diff --git a/crates/tor-config/Cargo.toml b/crates/tor-config/Cargo.toml index 7511573aa..a13c1a40e 100644 --- a/crates/tor-config/Cargo.toml +++ b/crates/tor-config/Cargo.toml @@ -65,6 +65,7 @@ void = "1" serde_json = "1.0.50" tempfile = "3" test-temp-dir = { version = "0.3.5", path = "../test-temp-dir" } +tor-basic-utils = { path = "../tor-basic-utils", version = "0.32.0", features = ["serde"] } tor-rtcompat = { path = "../tor-rtcompat", version = "0.32.0", features = ["tokio", "native-tls"] } tracing-test = "0.2.4" diff --git a/crates/tor-config/src/misc.rs b/crates/tor-config/src/misc.rs index 5b0a080ea..e26482837 100644 --- a/crates/tor-config/src/misc.rs +++ b/crates/tor-config/src/misc.rs @@ -242,6 +242,24 @@ impl<T: NotAutoValue> ExplicitOrAuto<T> { ExplicitOrAuto::Explicit(v) => Some(v), } } + + /// Maps an `ExplicitOrAuto<T>` to an `ExplicitOrAuto<U>` by applying a function to a contained + /// value. + pub fn map<U: NotAutoValue>(self, f: impl FnOnce(T) -> U) -> ExplicitOrAuto<U> { + match self { + Self::Auto => ExplicitOrAuto::Auto, + Self::Explicit(x) => ExplicitOrAuto::Explicit(f(x)), + } + } +} + +impl<T> From<T> for ExplicitOrAuto<T> +where + T: NotAutoValue, +{ + fn from(x: T) -> Self { + Self::Explicit(x) + } } /// A marker trait for types that do not serialize to the same value as [`ExplicitOrAuto::Auto`]. @@ -268,6 +286,9 @@ impl_not_auto_value_for_types!( bool ); +use tor_basic_utils::ByteQty; +impl_not_auto_value!(ByteQty); + // TODO implement `NotAutoValue` for other types too /// Padding enablement - rough amount of padding requested diff --git a/crates/tor-memquota/Cargo.toml b/crates/tor-memquota/Cargo.toml index 04dd1cd85..43bc4e826 100644 --- a/crates/tor-memquota/Cargo.toml +++ b/crates/tor-memquota/Cargo.toml @@ -12,6 +12,7 @@ categories = ["memory-management"] repository = "https://gitlab.torproject.org/tpo/core/arti.git/" [dependencies] +cfg-if = "1.0.0" derive-deftly = { version = "~1.1.0", features = ["full", "beta"] } derive_more = { version = "2.0.1", features = ["full"] } dyn-clone = "1.0.17" @@ -23,6 +24,7 @@ pin-project = "1" serde = { version = "1.0.103", features = ["derive"] } slotmap-careful = { path = "../slotmap-careful", version = "0.2.5" } static_assertions = "1" +sysinfo = "0.36.1" thiserror = "2" tor-async-utils = { path = "../tor-async-utils", version = "0.32.0" } tor-basic-utils = { path = "../tor-basic-utils", version = "0.32.0", features = ["serde"] } diff --git a/crates/tor-memquota/semver.md b/crates/tor-memquota/semver.md new file mode 100644 index 000000000..5678f0366 --- /dev/null +++ b/crates/tor-memquota/semver.md @@ -0,0 +1,4 @@ +BREAKING: `ConfigBuilder::max()` and `ConfigBuilder::low_water()` (which are effectively part of the + public "arti-client" API) now take a `impl Into<ExplicitOrAuto<usize>>` instead of a + `usize`. This should generally be backwards compatible, but may cause type inference + errors. diff --git a/crates/tor-memquota/src/config.rs b/crates/tor-memquota/src/config.rs index 763316b64..cd71a57b1 100644 --- a/crates/tor-memquota/src/config.rs +++ b/crates/tor-memquota/src/config.rs @@ -1,5 +1,10 @@ //! Configuration (private module) +use std::sync::LazyLock; + +use sysinfo::{MemoryRefreshKind, System}; +use tracing::warn; + use crate::internal_prelude::*; /// We want to support at least this many participants with a cache each @@ -21,13 +26,16 @@ define_derive_deftly! { impl ConfigBuilder { $( - ${when approx_equal($ftype, { Option::<Qty> })} + ${when approx_equal($ftype, { Option::<ExplicitOrAuto<Qty>> })} ${fattrs doc} /// /// (Setter method.) - pub fn $fname(&mut self, value: usize) -> &mut Self { - self.$fname = Some(Qty(value)); + // We use `value: impl Into<ExplicitOrAuto<usize>>` to avoid breaking users who used the + // previous `value: usize`. But this isn't 100% foolproof, for example if a user used + // `$fname(foo.into())`, which will fail type inference. + pub fn $fname(&mut self, value: impl Into<ExplicitOrAuto<usize>>) -> &mut Self { + self.$fname = Some(value.into().map(Qty)); self } ) @@ -54,17 +62,26 @@ pub struct Config(pub(crate) IfEnabled<ConfigInner>); pub struct ConfigBuilder { /// Maximum memory usage tolerated before reclamation starts /// - /// Setting this to `usize::MAX` disables the memory quota - /// (and that's the default). + /// Setting this to `usize::MAX` disables the memory quota. + /// + /// The default is "auto", + /// which uses a value derived from the total system memory. + /// It should not be assumed that the value used for "auto" + /// will remain stable across different versions of this library. /// /// Note that this is not a hard limit. /// See Approximate in [the overview](crate). - max: Option<Qty>, + max: Option<ExplicitOrAuto<Qty>>, /// Reclamation will stop when memory use is reduced to below this value /// - /// Default is 75% of the maximum. - low_water: Option<Qty>, + /// Default is "auto", which uses 75% of the maximum. + /// It should not be assumed that the value used for "auto" + /// will remain stable across different versions of this library. + /// + /// If set to an explicit value, + /// then `max` must be set to an explicit value as well. + low_water: Option<ExplicitOrAuto<Qty>>, } /// Configuration, if enabled @@ -77,7 +94,7 @@ pub struct ConfigBuilder { pub(crate) struct ConfigInner { /// Maximum memory usage /// - /// Guaranteed not to be `MAX`, since we're anbled + /// Guaranteed not to be `MAX`, since we're enabled pub max: Qty, /// Low water @@ -98,8 +115,9 @@ impl Config { /// /// Ad-hoc accessor for testing purposes. /// (ideally we'd use `visibility` to make fields `pub`, but that doesn't work.) - #[cfg(feature = "testing")] - pub fn inner(&self) -> Option<&ConfigInner> { + #[cfg(any(test, feature = "testing"))] + #[cfg_attr(feature = "testing", visibility::make(pub))] + fn inner(&self) -> Option<&ConfigInner> { self.0.as_ref().into_enabled() } } @@ -107,21 +125,38 @@ impl Config { impl ConfigBuilder { /// Builds a new `Config` from a builder /// - /// Returns an error unless at least `max` has been specified, - /// or if the fields values are invalid or inconsistent. + /// Returns an error if the fields values are invalid or inconsistent. pub fn build(&self) -> Result<Config, ConfigBuildError> { - let max = self.max.unwrap_or(Qty::MAX); + // both options default to "auto" + let max = self.max.unwrap_or(ExplicitOrAuto::Auto); + let low_water = self.low_water.unwrap_or(ExplicitOrAuto::Auto); - if max == Qty::MAX { - if self.low_water.is_some() { + // `MAX` indicates "disabled". + // TODO: Should we add a new "enabled" config option instead of using a sentinel value? + // But this would be a breaking change. Or maybe we should always enable the memquota + // machinery even if the user chooses an unreasonably large value, and not give users a way + // to disable it. + if max == ExplicitOrAuto::Explicit(Qty::MAX) { + // If it should be disabled, but the user provided an explicit value for `low_water`. + if matches!(low_water, ExplicitOrAuto::Explicit(_)) { return Err(ConfigBuildError::Inconsistent { fields: vec!["max".into(), "low_water".into()], - problem: "low_water supplied, but max omitted".into(), + problem: "low_water supplied, but max indicates that we should disable the memory quota".into(), }); }; return Ok(Config(IfEnabled::Noop)); } + // We don't want the user to set "auto" for `max`, but an explicit value for `low_water`. + // Otherwise this config is prone to breaking since a `max` of "auto" may change as system + // memory is removed (either physically or if running in a VM/container). + if matches!(max, ExplicitOrAuto::Auto) && matches!(low_water, ExplicitOrAuto::Explicit(_)) { + return Err(ConfigBuildError::Inconsistent { + fields: vec!["max".into(), "low_water".into()], + problem: "max is \"auto\", but low_water is set to an explicit quantity".into(), + }); + } + let enabled = EnabledToken::new_if_compiled_in() // .ok_or_else(|| ConfigBuildError::NoCompileTimeSupport { @@ -129,10 +164,18 @@ impl ConfigBuilder { problem: "cargo feature `memquota` disabled (in tor-memquota crate)".into(), })?; - let low_water = self.low_water.unwrap_or_else( - // - || Qty((*max as f32 * 0.75) as _), - ); + // The general logic is taken from c-tor (see `compute_real_max_mem_in_queues`). + // NOTE: Relays have an additional lower bound for explicitly given values (64 MiB), + // but we have no way of knowing whether we are a relay or not here. + let max = match max { + ExplicitOrAuto::Explicit(x) => x, + ExplicitOrAuto::Auto => compute_max_from_total_system_mem(total_available_memory()), + }; + + let low_water = match low_water { + ExplicitOrAuto::Explicit(x) => x, + ExplicitOrAuto::Auto => Qty((*max as f32 * 0.75) as _), + }; let config = ConfigInner { max, low_water }; @@ -160,6 +203,136 @@ impl ConfigBuilder { } } +/// Determine a max given the system's total available memory. +/// +/// This is used when `max` is configured as "auto". +/// It takes a `Result` so that we can handle the case where the total memory isn't available. +fn compute_max_from_total_system_mem(mem: Result<usize, MemQueryError>) -> Qty { + const MIB: usize = 1024 * 1024; + const GIB: usize = 1024 * 1024 * 1024; + + let mem = match mem { + Ok(x) => x, + Err(e) => { + warn!("Unable to get the total available memory. Using a constant max instead: {e}"); + + // Can't get the total available memory, + // so we return a max depending on whether the architecture is 32-bit or 64-bit. + return Qty({ + cfg_if::cfg_if! { + if #[cfg(target_pointer_width = "64")] { + 8 * GIB + } else { + 1 * GIB + } + } + }); + } + }; + + let mem = if mem >= 8 * GIB { + // From c-tor: + // + // > The idea behind this value is that the amount of RAM is more than enough + // > for a single relay and should allow the relay operator to run two relays + // > if they have additional bandwidth available. + (mem as f64 * 0.40) as usize + } else { + (mem as f64 * 0.75) as usize + }; + + // The (min, max) range to clamp `mem` to. + let clamp = { + cfg_if::cfg_if! { + if #[cfg(target_pointer_width = "64")] { + (256 * MIB, 8 * GIB) + } else { + (256 * MIB, 2 * GIB) + } + } + }; + + let mem = mem.clamp(clamp.0, clamp.1); + + Qty(mem) +} + +/// The total available memory in bytes. +/// +/// This is generally the amount of system RAM, +/// but we may also take into account other OS-specific limits such as cgroups. +/// +/// Returns `None` if we were unable to get the total available memory. +/// But see internal comments for details. +fn total_available_memory() -> Result<usize, MemQueryError> { + // The sysinfo crate says we should use only one `System` per application. + // But we're a library, so it's probably best to just make this global and reuse it. + // In reality getting the system memory probably shouldn't require persistent state, + // but since the internals of the sysinfo crate are opaque to us, + // we'll just follow their documentation and cache the `System`. + // + // NOTE: The sysinfo crate in practice gets more information than we ask for. + // For example `System::new()` will always query the `_SC_PAGESIZE` and `_SC_CLK_TCK` + // on Linux even though we only refresh the memory info below + // (see https://github.com/GuillaumeGomez/sysinfo/blob/fc31b411eea7b9983176399dc5be162786dec95b/src/unix/linux/system.rs#L152). + // This means that miri will fail to run on tests that build the config, even if the config uses + // explicit values. + static SYSTEM: LazyLock<Mutex<System>> = LazyLock::new(|| Mutex::new(System::new())); + let mut system = SYSTEM.lock().unwrap_or_else(|mut e| { + // The sysinfo crate has some internal panics which would poison this mutex. + // But we can easily reset it, rather than panicking ourselves if it's poisoned. + **e.get_mut() = System::new(); + SYSTEM.clear_poison(); + e.into_inner() + }); + + system.refresh_memory_specifics(MemoryRefreshKind::nothing().with_ram()); + + // It might be possible for 32-bit systems to return >usize::MAX due to PAE (I haven't looked + // into this), so we just saturate the value and don't consider this an error. + let mem = to_usize_saturating(system.total_memory()); + + // The sysinfo crate doesn't report errors, so the best we can do is guess that a value of 0 + // implies that it was unable to get the total memory. + // + // We also need to return early to prevent a panic below. + if mem == 0 { + return Err(MemQueryError::Unavailable); + } + + // Note: The docs for the sysinfo crate say: + // + // > You need to have run refresh_memory at least once before calling this method. + // + // But as implemented, it also panics if `sys.mem_total == 0` (for example if the refresh + // silently failed). + let Some(cgroups) = system.cgroup_limits() else { + // There is no cgroup (or we're a non-Linux platform). + return Ok(mem); + }; + + // The `cgroup_limits()` surprisingly doesn't actually return the unaltered cgroups limits. + // It also adjusts them depending on the total memory. + // Since this is all undocumented, we'll also do the same calculation here. + let mem = std::cmp::min(mem, to_usize_saturating(cgroups.total_memory)); + + Ok(mem) +} + +/// An error when we are unable to obtain the system's total available memory. +#[derive(Clone, Debug, thiserror::Error)] +enum MemQueryError { + /// The total available memory is unavailable. + #[error("total available memory is unavailable")] + Unavailable, +} + +/// Convert a `u64` to a `usize`, saturating if the value would overflow. +fn to_usize_saturating(x: u64) -> usize { + // this will be optimized to a no-op on 64-bit systems + x.try_into().unwrap_or(usize::MAX) +} + #[cfg(test)] mod test { // @@ begin test lint list maintained by maint/add_warning @@ @@ -180,6 +353,9 @@ mod test { use serde_json::json; #[test] + // A value of "auto" depends on the system memory, + // which typically results in libc calls or syscall that aren't supported by miri. + #[cfg_attr(miri, ignore)] fn configs() { let chk_ok_raw = |j, c| { let b: ConfigBuilder = serde_json::from_value(j).unwrap(); @@ -215,17 +391,119 @@ mod test { chk_err(j, "UNSUPPORTED"); }; + let chk_builds = |j| { + cfg_if::cfg_if! { + if #[cfg(feature = "memquota")] { + let b: ConfigBuilder = serde_json::from_value(j).unwrap(); + b.build().unwrap(); + } else { + chk_err(j, "UNSUPPORTED"); + } + } + }; + chk_ok(json! {{ "max": "8 MiB" }}, 8, 6); + chk_ok(json! {{ "max": "8 MiB", "low_water": "auto" }}, 8, 6); chk_ok(json! {{ "max": "8 MiB", "low_water": "4 MiB" }}, 8, 4); - chk_ok_raw(json! {{ }}, Config(IfEnabled::Noop)); + + // We don't know what the exact values will be since they are derived from the system + // memory. + chk_builds(json! {{ }}); + chk_builds(json! {{ "max": "auto" }}); + chk_builds(json! {{ "low_water": "auto" }}); + chk_builds(json! {{ "max": "auto", "low_water": "auto" }}); chk_err( json! {{ "low_water": "4 MiB" }}, - "low_water supplied, but max omitted", + "max is \"auto\", but low_water is set to an explicit quantity", ); chk_err( json! {{ "max": "8 MiB", "low_water": "8 MiB" }}, "inconsistent: low_water / max", ); + + // `usize::MAX` is a special value. + chk_err( + json! {{ "max": usize::MAX.to_string(), "low_water": "8 MiB" }}, + "low_water supplied, but max indicates that we should disable the memory quota", + ); + chk_builds(json! {{ "max": (usize::MAX - 1).to_string(), "low_water": "8 MiB" }}); + + // check that the builder works as expected + #[cfg(feature = "memquota")] + { + let mut b = Config::builder(); + b.max(ExplicitOrAuto::Explicit(100_000_000)); + if let Some(inner) = b.build().unwrap().inner() { + assert_eq!(inner.max, Qty(100_000_000)); + } + + let mut b = Config::builder(); + b.max(100_000_000); + if let Some(inner) = b.build().unwrap().inner() { + assert_eq!(inner.max, Qty(100_000_000)); + } + + let mut b = ConfigBuilder::default(); + b.max(ExplicitOrAuto::Auto); + b.build().unwrap(); + } + } + + /// Test the logic that computes the `max` when configured as "auto". + #[test] + // We do some `1 * X` operations below for readability. + #[allow(clippy::identity_op)] + fn auto_max() { + #[allow(unused)] + fn check_helper(val: Qty, expected_32: Qty, expected_64: Qty) { + assert_eq!(val, { + cfg_if::cfg_if! { + if #[cfg(target_pointer_width = "64")] { + expected_64 + } else if #[cfg(target_pointer_width = "32")] { + expected_32 + } else { + panic!("Unsupported architecture :("); + } + } + }); + } + + check_helper( + compute_max_from_total_system_mem(Err(MemQueryError::Unavailable)), + /* 32-bit */ Qty(1 * 1024 * 1024 * 1024), + /* 64-bit */ Qty(8 * 1024 * 1024 * 1024), + ); + check_helper( + compute_max_from_total_system_mem(Ok(8 * 1024 * 1024 * 1024)), + /* 32-bit */ Qty(2 * 1024 * 1024 * 1024), + /* 64-bit */ Qty(3435973836), + ); + check_helper( + compute_max_from_total_system_mem(Ok(7 * 1024 * 1024 * 1024)), + /* 32-bit */ Qty(2 * 1024 * 1024 * 1024), + /* 64-bit */ Qty(5637144576), + ); + check_helper( + compute_max_from_total_system_mem(Ok(1 * 1024 * 1024 * 1024)), + /* 32-bit */ Qty(805306368), + /* 64-bit */ Qty(805306368), + ); + check_helper( + compute_max_from_total_system_mem(Ok(7 * 1024)), + /* 32-bit */ Qty(256 * 1024 * 1024), + /* 64-bit */ Qty(256 * 1024 * 1024), + ); + check_helper( + compute_max_from_total_system_mem(Ok(0)), + /* 32-bit */ Qty(256 * 1024 * 1024), + /* 64-bit */ Qty(256 * 1024 * 1024), + ); + check_helper( + compute_max_from_total_system_mem(Ok(usize::MAX)), + /* 32-bit */ Qty(2 * 1024 * 1024 * 1024), + /* 64-bit */ Qty(8 * 1024 * 1024 * 1024), + ); } } diff --git a/crates/tor-memquota/src/internal_prelude.rs b/crates/tor-memquota/src/internal_prelude.rs index 5bddc6466..c4c358885 100644 --- a/crates/tor-memquota/src/internal_prelude.rs +++ b/crates/tor-memquota/src/internal_prelude.rs @@ -55,7 +55,7 @@ pub(crate) use { tor_async_utils::mpsc_channel_no_memquota, tor_async_utils::stream_peek::StreamUnobtrusivePeeker, tor_basic_utils::ByteQty as Qty, - tor_config::{ConfigBuildError, ReconfigureError}, + tor_config::{ConfigBuildError, ExplicitOrAuto, ReconfigureError}, tor_error::{error_report, internal, into_internal, trace_report, Bug, ErrorKind, HasKind}, tor_log_ratelim::log_ratelim, tor_rtcompat::{CoarseInstant, CoarseTimeProvider, DynTimeProvider}, diff --git a/crates/tor-memquota/src/mtracker.rs b/crates/tor-memquota/src/mtracker.rs index b3569376e..e2a8f053c 100644 --- a/crates/tor-memquota/src/mtracker.rs +++ b/crates/tor-memquota/src/mtracker.rs @@ -568,7 +568,7 @@ impl MemoryQuotaTracker { let for_task = Arc::downgrade(&tracker); runtime.spawn(reclaim::task(for_task, reclaim_rx, enabled))?; - info!(%max, %low_water, "memory quota tracking initialised"); + info!(%max, %low_water, "Memory quota tracking initialised"); Ok(tracker) } diff --git a/maint/downgrade_dependencies b/maint/downgrade_dependencies index 3a7b5a965..1e68d6448 100755 --- a/maint/downgrade_dependencies +++ b/maint/downgrade_dependencies @@ -21,5 +21,8 @@ $CARGO update \ -p zeroize_derive:1.3.2 \ -p filetime \ -p tap:1.0.0 \ - -p event-listener:3.0.0 \ -p paste + +# The downgraded 'async-global-executor' lib specifies a dependency of 'futures-lite' 1.0, +# but uses the `future::or` function which wasn't available until 'futures-lite' 1.3.0. +$CARGO update --precise 1.3.0 [email protected] |
