summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--Cargo.lock44
-rw-r--r--crates/arti-client/Cargo.toml9
-rw-r--r--crates/arti-client/README.md9
-rw-r--r--crates/arti-relay/Cargo.toml3
-rw-r--r--crates/arti/Cargo.toml6
-rw-r--r--crates/arti/README.md11
-rw-r--r--crates/arti/build.rs4
-rw-r--r--crates/arti/src/arti-example-config.toml21
-rw-r--r--crates/arti/src/cfg.rs48
-rw-r--r--crates/tor-config/Cargo.toml1
-rw-r--r--crates/tor-config/src/misc.rs21
-rw-r--r--crates/tor-memquota/Cargo.toml2
-rw-r--r--crates/tor-memquota/semver.md4
-rw-r--r--crates/tor-memquota/src/config.rs324
-rw-r--r--crates/tor-memquota/src/internal_prelude.rs2
-rw-r--r--crates/tor-memquota/src/mtracker.rs2
-rwxr-xr-xmaint/downgrade_dependencies5
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]