diff --git a/nmrs/CHANGELOG.md b/nmrs/CHANGELOG.md index 9d9a0927..1f0b6b98 100644 --- a/nmrs/CHANGELOG.md +++ b/nmrs/CHANGELOG.md @@ -3,6 +3,8 @@ All notable changes to the `nmrs` crate will be documented in this file. ## [Unreleased] +### Added +- Concurrency protection ([#268](https://github.com/cachebag/nmrs/pull/268)) ### Changed - Convert BDADDR to BlueZ device path via `bluez_device_path` helper ([#266](https://github.com/cachebag/nmrs/pull/266)) diff --git a/nmrs/src/api/models.rs b/nmrs/src/api/models.rs index 87de7dab..06d4cd26 100644 --- a/nmrs/src/api/models.rs +++ b/nmrs/src/api/models.rs @@ -1965,8 +1965,16 @@ pub enum DeviceState { Disconnected, /// Device is preparing to connect. Prepare, - /// Device is being configured (IP, etc.). + /// Device is being configured. Config, + /// Device requires authentication credentials. + NeedAuth, + /// Device is requesting IP configuration. + IpConfig, + /// Device is verifying IP connectivity. + IpCheck, + /// Device is waiting for secondary connections. + Secondaries, /// Device is fully connected and operational. Activated, /// Device is disconnecting. @@ -1977,6 +1985,27 @@ pub enum DeviceState { Other(u32), } +impl DeviceState { + /// Returns `true` if the device is in a transitional (in-progress) state. + /// + /// Transitional states indicate an active connection or disconnection + /// operation: Prepare, Config, NeedAuth, IpConfig, IpCheck, Secondaries, + /// or Deactivating. + #[must_use] + pub fn is_transitional(&self) -> bool { + matches!( + self, + Self::Prepare + | Self::Config + | Self::NeedAuth + | Self::IpConfig + | Self::IpCheck + | Self::Secondaries + | Self::Deactivating + ) + } +} + impl Device { /// Returns `true` if this is a wired (Ethernet) device. #[must_use] @@ -2369,15 +2398,19 @@ impl From for DeviceType { impl From for DeviceState { fn from(value: u32) -> Self { match value { - 10 => DeviceState::Unmanaged, - 20 => DeviceState::Unavailable, - 30 => DeviceState::Disconnected, - 40 => DeviceState::Prepare, - 50 => DeviceState::Config, - 100 => DeviceState::Activated, - 110 => DeviceState::Deactivating, - 120 => DeviceState::Failed, - v => DeviceState::Other(v), + 10 => Self::Unmanaged, + 20 => Self::Unavailable, + 30 => Self::Disconnected, + 40 => Self::Prepare, + 50 => Self::Config, + 60 => Self::NeedAuth, + 70 => Self::IpConfig, + 80 => Self::IpCheck, + 90 => Self::Secondaries, + 100 => Self::Activated, + 110 => Self::Deactivating, + 120 => Self::Failed, + v => Self::Other(v), } } } @@ -2402,15 +2435,19 @@ impl Display for DeviceType { impl Display for DeviceState { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match self { - DeviceState::Unmanaged => write!(f, "Unmanaged"), - DeviceState::Unavailable => write!(f, "Unavailable"), - DeviceState::Disconnected => write!(f, "Disconnected"), - DeviceState::Prepare => write!(f, "Preparing"), - DeviceState::Config => write!(f, "Configuring"), - DeviceState::Activated => write!(f, "Activated"), - DeviceState::Deactivating => write!(f, "Deactivating"), - DeviceState::Failed => write!(f, "Failed"), - DeviceState::Other(v) => write!(f, "Other({v})"), + Self::Unmanaged => write!(f, "Unmanaged"), + Self::Unavailable => write!(f, "Unavailable"), + Self::Disconnected => write!(f, "Disconnected"), + Self::Prepare => write!(f, "Preparing"), + Self::Config => write!(f, "Configuring"), + Self::NeedAuth => write!(f, "NeedAuth"), + Self::IpConfig => write!(f, "IpConfig"), + Self::IpCheck => write!(f, "IpCheck"), + Self::Secondaries => write!(f, "Secondaries"), + Self::Activated => write!(f, "Activated"), + Self::Deactivating => write!(f, "Deactivating"), + Self::Failed => write!(f, "Failed"), + Self::Other(v) => write!(f, "Other({v})"), } } } @@ -3422,4 +3459,46 @@ mod tests { assert_eq!(config1.connection_timeout, Duration::from_secs(120)); assert_eq!(config2.connection_timeout, Duration::from_secs(120)); } + + #[test] + fn test_device_state_is_transitional() { + let transitional = [ + DeviceState::Prepare, + DeviceState::Config, + DeviceState::NeedAuth, + DeviceState::IpConfig, + DeviceState::IpCheck, + DeviceState::Secondaries, + DeviceState::Deactivating, + ]; + for state in &transitional { + assert!(state.is_transitional(), "{state:?} should be transitional"); + } + + let stable = [ + DeviceState::Unmanaged, + DeviceState::Unavailable, + DeviceState::Disconnected, + DeviceState::Activated, + DeviceState::Failed, + DeviceState::Other(999), + ]; + for state in &stable { + assert!( + !state.is_transitional(), + "{state:?} should not be transitional" + ); + } + } + + #[test] + fn test_device_state_from_u32_intermediate_states() { + assert_eq!(DeviceState::from(40), DeviceState::Prepare); + assert_eq!(DeviceState::from(50), DeviceState::Config); + assert_eq!(DeviceState::from(60), DeviceState::NeedAuth); + assert_eq!(DeviceState::from(70), DeviceState::IpConfig); + assert_eq!(DeviceState::from(80), DeviceState::IpCheck); + assert_eq!(DeviceState::from(90), DeviceState::Secondaries); + assert_eq!(DeviceState::from(110), DeviceState::Deactivating); + } } diff --git a/nmrs/src/api/network_manager.rs b/nmrs/src/api/network_manager.rs index d4fb5131..7c7ea3b8 100644 --- a/nmrs/src/api/network_manager.rs +++ b/nmrs/src/api/network_manager.rs @@ -11,7 +11,8 @@ use crate::core::connection_settings::{ get_saved_connection_path, has_saved_connection, list_saved_connections, }; use crate::core::device::{ - list_bluetooth_devices, list_devices, set_wifi_enabled, wait_for_wifi_ready, wifi_enabled, + is_connecting, list_bluetooth_devices, list_devices, set_wifi_enabled, wait_for_wifi_ready, + wifi_enabled, }; use crate::core::scan::{current_network, list_networks, scan_networks}; use crate::core::vpn::{connect_vpn, disconnect_vpn, get_vpn_info, list_vpn_connections}; @@ -114,6 +115,14 @@ use crate::Result; /// /// `NetworkManager` is `Clone` and can be safely shared across async tasks. /// Each clone shares the same underlying D-Bus connection. +/// +/// # Concurrency +/// +/// Concurrent connection operations (e.g. calling [`connect`](Self::connect) +/// from multiple tasks simultaneously) are **not supported** and may cause +/// race conditions. Use [`is_connecting`](Self::is_connecting) to check +/// whether a connection operation is already in progress before starting +/// a new one. #[derive(Debug, Clone)] pub struct NetworkManager { conn: Connection, @@ -425,6 +434,32 @@ impl NetworkManager { scan_networks(&self.conn).await } + /// Returns whether any network device is currently in a transitional state. + /// + /// A device is considered "connecting" when its state is one of: + /// Prepare, Config, NeedAuth, IpConfig, IpCheck, Secondaries, or Deactivating. + /// + /// Use this to guard against concurrent connection attempts, which are + /// not supported and may cause undefined behavior. + /// + /// # Example + /// + /// ```no_run + /// use nmrs::NetworkManager; + /// + /// # async fn example() -> nmrs::Result<()> { + /// let nm = NetworkManager::new().await?; + /// + /// if nm.is_connecting().await? { + /// eprintln!("A connection operation is already in progress"); + /// } + /// # Ok(()) + /// # } + /// ``` + pub async fn is_connecting(&self) -> Result { + is_connecting(&self.conn).await + } + /// Check if a network is connected pub async fn is_connected(&self, ssid: &str) -> Result { is_connected(&self.conn, ssid).await diff --git a/nmrs/src/core/device.rs b/nmrs/src/core/device.rs index 58a9a1ca..6a9b5f36 100644 --- a/nmrs/src/core/device.rs +++ b/nmrs/src/core/device.rs @@ -134,6 +134,37 @@ pub(crate) async fn list_devices(conn: &Connection) -> Result> { Ok(devices) } +/// Returns `true` if any network device is in a transitional state +/// (preparing, configuring, authenticating, obtaining IP, etc.). +/// +/// Useful for guarding against concurrent connection attempts. +pub(crate) async fn is_connecting(conn: &Connection) -> Result { + let nm = NMProxy::new(conn).await?; + let devices = nm.get_devices().await?; + + for dp in devices { + let dev = NMDeviceProxy::builder(conn) + .path(dp.clone())? + .build() + .await?; + + let raw_state = dev + .state() + .await + .map_err(|e| ConnectionError::DbusOperation { + context: format!("failed to get state for device {}", dp.as_str()), + source: e, + })?; + + let state: DeviceState = raw_state.into(); + if state.is_transitional() { + return Ok(true); + } + } + + Ok(false) +} + pub(crate) async fn list_bluetooth_devices(conn: &Connection) -> Result> { let proxy = NMProxy::new(conn).await?; let paths = proxy.get_devices().await?; diff --git a/nmrs/src/types/constants.rs b/nmrs/src/types/constants.rs index b0b35d2f..63dbb344 100644 --- a/nmrs/src/types/constants.rs +++ b/nmrs/src/types/constants.rs @@ -12,11 +12,20 @@ pub mod device_type { // pub const LOOPBACK: u32 = 32; } -/// NetworkManager device state constants +/// NetworkManager device state constants. +/// +/// Reference: pub mod device_state { pub const UNAVAILABLE: u32 = 20; pub const DISCONNECTED: u32 = 30; + // pub const PREPARE: u32 = 40; + // pub const CONFIG: u32 = 50; + // pub const NEED_AUTH: u32 = 60; + // pub const IP_CONFIG: u32 = 70; + // pub const IP_CHECK: u32 = 80; + // pub const SECONDARIES: u32 = 90; pub const ACTIVATED: u32 = 100; + // pub const DEACTIVATING: u32 = 110; } /// WiFi security flag constants diff --git a/nmrs/tests/integration_test.rs b/nmrs/tests/integration_test.rs index 1e67efb3..27d114a6 100644 --- a/nmrs/tests/integration_test.rs +++ b/nmrs/tests/integration_test.rs @@ -567,12 +567,14 @@ async fn test_device_states() { | DeviceState::Disconnected | DeviceState::Prepare | DeviceState::Config + | DeviceState::NeedAuth + | DeviceState::IpConfig + | DeviceState::IpCheck + | DeviceState::Secondaries | DeviceState::Activated | DeviceState::Deactivating | DeviceState::Failed - | DeviceState::Other(_) => { - // Valid state - } + | DeviceState::Other(_) => {} _ => { panic!("Invalid device state: {:?}", device.state); }