Merge branch 'bug299' into 'main'

Make max_file_limit configurable

Closes #299

See merge request tpo/core/arti!261

crates/arti-client/src/config.rs

@@ -224,6 +224,46 @@ impl From<StorageConfig> for StorageConfigBuilder {
     }
 }
 
+/// Configuration for system resources used by Tor.
+///
+/// You cannot change this section on a running Arti client.
+#[derive(Deserialize, Debug, Clone, Builder, Eq, PartialEq)]
+#[serde(deny_unknown_fields)]
+#[builder(build_fn(error = "ConfigBuildError"))]
+#[non_exhaustive]
+pub struct SystemConfig {
+    /// Maximum number of file descriptors we should launch with
+    #[builder(setter(into), default = "default_max_files()")]
+    #[serde(default = "default_max_files")]
+    pub max_files: u64,
+}
+
+/// Return the default maximum number of file descriptors to launch with.
+fn default_max_files() -> u64 {
+    16384
+}
+
+impl Default for SystemConfig {
+    fn default() -> Self {
+        Self::builder().build().expect("Default builder failed")
+    }
+}
+
+impl SystemConfig {
+    /// Return a new SystemConfigBuilder.
+    pub fn builder() -> SystemConfigBuilder {
+        SystemConfigBuilder::default()
+    }
+}
+
+impl From<SystemConfig> for SystemConfigBuilder {
+    fn from(cfg: SystemConfig) -> SystemConfigBuilder {
+        let mut builder = SystemConfigBuilder::default();
+        builder.max_files(cfg.max_files);
+        builder
+    }
+}
+
 /// A configuration used to bootstrap a [`TorClient`](crate::TorClient).
 ///
 /// In order to connect to the Tor network, Arti needs to know a few
@@ -273,6 +313,9 @@ pub struct TorClientConfig {
 
     /// Information about timing out client requests.
     pub(crate) stream_timeouts: StreamTimeoutConfig,
+
+    /// Information about system resources
+    pub system: SystemConfig,
 }
 
 impl Default for TorClientConfig {
@@ -336,6 +379,8 @@ pub struct TorClientConfigBuilder {
     address_filter: ClientAddrConfigBuilder,
     /// Inner builder for the `stream_timeouts` section.
     stream_timeouts: StreamTimeoutConfigBuilder,
+    /// Inner builder for the `system` section.
+    system: SystemConfigBuilder,
 }
 
 impl TorClientConfigBuilder {
@@ -371,6 +416,7 @@ impl TorClientConfigBuilder {
             .stream_timeouts
             .build()
             .map_err(|e| e.within("stream_timeouts"))?;
+        let system = self.system.build().map_err(|e| e.within("system"))?;
 
         Ok(TorClientConfig {
             tor_network,
@@ -382,6 +428,7 @@ impl TorClientConfigBuilder {
             circuit_timing,
             address_filter,
             stream_timeouts,
+            system,
         })
     }
 
@@ -485,6 +532,13 @@ impl TorClientConfigBuilder {
     pub fn address_filter(&mut self) -> &mut ClientAddrConfigBuilder {
         &mut self.address_filter
     }
+
+    /// Return a mutable reference to a [`SystemConfigBuilder`].
+    ///
+    /// This section is used to configure the system resources used by Arti.
+    pub fn system(&mut self) -> &mut SystemConfigBuilder {
+        &mut self.system
+    }
 }
 
 impl From<TorClientConfig> for TorClientConfigBuilder {
@@ -499,6 +553,7 @@ impl From<TorClientConfig> for TorClientConfigBuilder {
             circuit_timing,
             address_filter,
             stream_timeouts,
+            system,
         } = cfg;
 
         TorClientConfigBuilder {
@@ -511,6 +566,7 @@ impl From<TorClientConfig> for TorClientConfigBuilder {
             circuit_timing: circuit_timing.into(),
             address_filter: address_filter.into(),
             stream_timeouts: stream_timeouts.into(),
+            system: system.into(),
         }
     }
 }

crates/arti-config/src/arti_defaults.toml

@@ -164,3 +164,10 @@ resolve_timeout = "10 sec"
 
 # How long should we wait before timing out when resolving a DNS PTR record?
 resolve_ptr_timeout = "10 sec"
+
+# Configuration for the system resources used by Arti.
+[system]
+
+# What is the maximum number of file descriptors which should be available
+# to Arti when we launch?
+max_files = 16384

crates/arti-config/src/options.rs

@@ -4,7 +4,8 @@ use arti_client::config::{
     circ,
     dir::{self, DownloadScheduleConfig, NetworkConfig},
     ClientAddrConfig, ClientAddrConfigBuilder, StorageConfig, StorageConfigBuilder,
-    StreamTimeoutConfig, StreamTimeoutConfigBuilder, TorClientConfig, TorClientConfigBuilder,
+    StreamTimeoutConfig, StreamTimeoutConfigBuilder, SystemConfig, SystemConfigBuilder,
+    TorClientConfig, TorClientConfigBuilder,
 };
 use derive_builder::Builder;
 use serde::Deserialize;
@@ -247,6 +248,9 @@ pub struct ArtiConfig {
 
     /// Information about when to time out client requests.
     stream_timeouts: StreamTimeoutConfig,
+
+    /// Information on system resources used by Arti.
+    system: SystemConfig,
 }
 
 impl From<ArtiConfig> for TorClientConfigBuilder {
@@ -328,6 +332,8 @@ pub struct ArtiConfigBuilder {
     address_filter: ClientAddrConfigBuilder,
     /// Builder for the stream timeout rules.
     stream_timeouts: StreamTimeoutConfigBuilder,
+    /// Builder for system resource configuration.
+    system: SystemConfigBuilder,
 }
 
 impl ArtiConfigBuilder {
@@ -365,6 +371,7 @@ impl ArtiConfigBuilder {
             .stream_timeouts
             .build()
             .map_err(|e| e.within("stream_timeouts"))?;
+        let system = self.system.build().map_err(|e| e.within("system"))?;
         Ok(ArtiConfig {
             proxy,
             logging,
@@ -377,6 +384,7 @@ impl ArtiConfigBuilder {
             circuit_timing,
             address_filter,
             stream_timeouts,
+            system,
         })
     }
 
@@ -476,6 +484,13 @@ impl ArtiConfigBuilder {
     pub fn stream_timeouts(&mut self) -> &mut StreamTimeoutConfigBuilder {
         &mut self.stream_timeouts
     }
+
+    /// Return a mutable reference to a [`SystemConfigBuilder`].
+    ///
+    /// This section controls the system parameters used by Arti.
+    pub fn system(&mut self) -> &mut SystemConfigBuilder {
+        &mut self.system
+    }
 }
 
 impl From<ArtiConfig> for ArtiConfigBuilder {
@@ -492,6 +507,7 @@ impl From<ArtiConfig> for ArtiConfigBuilder {
             circuit_timing: cfg.circuit_timing.into(),
             address_filter: cfg.address_filter.into(),
             stream_timeouts: cfg.stream_timeouts.into(),
+            system: cfg.system.into(),
         }
     }
 }

crates/arti/src/main.rs

@@ -227,7 +227,7 @@ fn main() -> Result<()> {
             socks_port
         );
 
-        process::use_max_file_limit();
+        process::use_max_file_limit(&client_config);
 
         cfg_if::cfg_if! {
             if #[cfg(all(feature="tokio", feature="native-tls"))] {

crates/arti/src/process.rs

@@ -1,5 +1,7 @@
 //! Code to adjust process-related parameters.
 
+use arti_client::TorClientConfig;
+
 /// Set our current maximum-file limit to a large value, if we can.
 ///
 /// Since we're going to be used as a proxy, we're likely to need a
@@ -7,18 +9,9 @@
 ///
 /// # Limitations
 ///
-/// Maybe this should take a value from the configuration instead.
-///
 /// This doesn't actually do anything on windows.
-pub(crate) fn use_max_file_limit() {
-    /// Default maximum value to set for our maximum-file limit.
-    ///
-    /// If the system supports more than this, we won't ask for it.
-    /// This should be plenty for proxy usage, though relays and onion
-    /// services (once supported) may need more.
-    const DFLT_MAX_N_FILES: u64 = 16384;
-
-    match rlimit::utils::increase_nofile_limit(DFLT_MAX_N_FILES) {
+pub(crate) fn use_max_file_limit(config: &TorClientConfig) {
+    match rlimit::utils::increase_nofile_limit(config.system.max_files) {
         Ok(n) => tracing::debug!("Increased process file limit to {}", n),
         Err(e) => tracing::warn!("Error while increasing file limit: {}", e),
     }