From 33195b29ac5506f663b638a5ae258d929ce0170c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Thu, 13 Jul 2023 12:10:22 +0100 Subject: [PATCH] Use an async-aware mutex --- rust/Cargo.lock | 2 +- rust/Cargo.toml | 1 + rust/agama-dbus-server/Cargo.toml | 4 +- .../src/network/dbus/interfaces.rs | 126 +++++++++--------- .../src/network/dbus/tree.rs | 12 +- 5 files changed, 73 insertions(+), 72 deletions(-) diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 6afaee1473..e9d73fbda1 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -35,8 +35,8 @@ dependencies = [ "agama-locale-data", "anyhow", "async-std", + "futures", "log", - "parking_lot", "serde", "serde_yaml", "simplelog", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index 002eba41ec..6e2475d875 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -7,3 +7,4 @@ members = [ "agama-locale-data", "agama-settings" ] + diff --git a/rust/agama-dbus-server/Cargo.toml b/rust/agama-dbus-server/Cargo.toml index f74f49d0d2..97fca7575e 100644 --- a/rust/agama-dbus-server/Cargo.toml +++ b/rust/agama-dbus-server/Cargo.toml @@ -16,7 +16,7 @@ zbus = "3.7.0" zbus_macros = "3.7.0" async-std = { version = "1.12.0", features = ["attributes"]} uuid = { version = "1.3.4", features = ["v4"] } -parking_lot = "0.12.1" thiserror = "1.0.40" serde = { version = "1.0.152", features = ["derive"] } -serde_yaml = "0.9.24" \ No newline at end of file +serde_yaml = "0.9.24" +futures = "0.3.28" diff --git a/rust/agama-dbus-server/src/network/dbus/interfaces.rs b/rust/agama-dbus-server/src/network/dbus/interfaces.rs index db5e2ad118..c4cc84d0c1 100644 --- a/rust/agama-dbus-server/src/network/dbus/interfaces.rs +++ b/rust/agama-dbus-server/src/network/dbus/interfaces.rs @@ -13,7 +13,7 @@ use crate::network::{ use log; use agama_lib::network::types::SSID; -use parking_lot::{MappedMutexGuard, Mutex, MutexGuard}; +use futures::lock::{MappedMutexGuard, Mutex, MutexGuard}; use std::{ net::{AddrParseError, Ipv4Addr}, sync::{mpsc::Sender, Arc}, @@ -42,8 +42,8 @@ impl Devices { #[dbus_interface(name = "org.opensuse.Agama.Network1.Devices")] impl Devices { /// Returns the D-Bus paths of the network devices. - pub fn get_devices(&self) -> Vec { - let objects = self.objects.lock(); + pub async fn get_devices(&self) -> Vec { + let objects = self.objects.lock().await; objects .devices_paths() .iter() @@ -112,8 +112,8 @@ impl Connections { #[dbus_interface(name = "org.opensuse.Agama.Network1.Connections")] impl Connections { /// Returns the D-Bus paths of the network connections. - pub fn get_connections(&self) -> Vec { - let objects = self.objects.lock(); + pub async fn get_connections(&self) -> Vec { + let objects = self.objects.lock().await; objects .connections_paths() .iter() @@ -126,7 +126,7 @@ impl Connections { /// * `id`: connection name. /// * `ty`: connection type (see [agama_lib::network::types::DeviceType]). pub async fn add_connection(&mut self, id: String, ty: u8) -> zbus::fdo::Result<()> { - let actions = self.actions.lock(); + let actions = self.actions.lock().await; actions .send(Action::AddConnection(id, ty.try_into()?)) .unwrap(); @@ -137,7 +137,7 @@ impl Connections { /// /// * `id`: connection ID. pub async fn get_connection(&self, id: &str) -> zbus::fdo::Result { - let objects = self.objects.lock(); + let objects = self.objects.lock().await; match objects.connection_path(id) { Some(path) => Ok(path.into()), None => Err(NetworkStateError::UnknownConnection(id.to_string()).into()), @@ -148,7 +148,7 @@ impl Connections { /// /// * `uuid`: connection UUID.. pub async fn remove_connection(&mut self, id: &str) -> zbus::fdo::Result<()> { - let actions = self.actions.lock(); + let actions = self.actions.lock().await; actions .send(Action::RemoveConnection(id.to_string())) .unwrap(); @@ -159,7 +159,7 @@ impl Connections { /// /// It includes adding, updating and removing connections as needed. pub async fn apply(&self) -> zbus::fdo::Result<()> { - let actions = self.actions.lock(); + let actions = self.actions.lock().await; actions.send(Action::Apply).unwrap(); Ok(()) } @@ -181,8 +181,8 @@ impl Connection { } /// Returns the underlying connection. - fn get_connection(&self) -> MutexGuard { - self.connection.lock() + async fn get_connection(&self) -> MutexGuard { + self.connection.lock().await } } @@ -194,8 +194,8 @@ impl Connection { /// backend. For instance, when using NetworkManager (which is the only supported backend by /// now), it uses the original ID but appending a number in case the ID is duplicated. #[dbus_interface(property)] - pub fn id(&self) -> String { - self.get_connection().id().to_string() + pub async fn id(&self) -> String { + self.get_connection().await.id().to_string() } } @@ -218,18 +218,18 @@ impl Ipv4 { } /// Returns the underlying connection. - fn get_connection(&self) -> MutexGuard { - self.connection.lock() + async fn get_connection(&self) -> MutexGuard { + self.connection.lock().await } /// Updates the connection data in the NetworkSystem. /// /// * `connection`: Updated connection. - fn update_connection( + async fn update_connection<'a>( &self, - connection: MutexGuard, + connection: MutexGuard<'a, NetworkConnection>, ) -> zbus::fdo::Result<()> { - let actions = self.actions.lock(); + let actions = self.actions.lock().await; actions .send(Action::UpdateConnection(connection.clone())) .unwrap(); @@ -243,8 +243,8 @@ impl Ipv4 { /// /// When the method is 'auto', these addresses are used as additional addresses. #[dbus_interface(property)] - pub fn addresses(&self) -> Vec { - let connection = self.get_connection(); + pub async fn addresses(&self) -> Vec { + let connection = self.get_connection().await; connection .ipv4() .addresses @@ -254,8 +254,8 @@ impl Ipv4 { } #[dbus_interface(property)] - pub fn set_addresses(&mut self, addresses: Vec) -> zbus::fdo::Result<()> { - let mut connection = self.get_connection(); + pub async fn set_addresses(&mut self, addresses: Vec) -> zbus::fdo::Result<()> { + let mut connection = self.get_connection().await; let parsed: Vec = addresses .into_iter() .filter_map(|ip| match ip.parse::() { @@ -267,7 +267,7 @@ impl Ipv4 { }) .collect(); connection.ipv4_mut().addresses = parsed; - self.update_connection(connection) + self.update_connection(connection).await } /// IP configuration method. @@ -276,22 +276,22 @@ impl Ipv4 { /// /// See [crate::network::model::IpMethod]. #[dbus_interface(property)] - pub fn method(&self) -> String { - let connection = self.get_connection(); + pub async fn method(&self) -> String { + let connection = self.get_connection().await; connection.ipv4().method.to_string() } #[dbus_interface(property)] - pub fn set_method(&mut self, method: &str) -> zbus::fdo::Result<()> { - let mut connection = self.get_connection(); + pub async fn set_method(&mut self, method: &str) -> zbus::fdo::Result<()> { + let mut connection = self.get_connection().await; connection.ipv4_mut().method = method.parse()?; - self.update_connection(connection) + self.update_connection(connection).await } /// Name server addresses. #[dbus_interface(property)] - pub fn nameservers(&self) -> Vec { - let connection = self.get_connection(); + pub async fn nameservers(&self) -> Vec { + let connection = self.get_connection().await; connection .ipv4() .nameservers @@ -301,8 +301,8 @@ impl Ipv4 { } #[dbus_interface(property)] - pub fn set_nameservers(&mut self, addresses: Vec) -> zbus::fdo::Result<()> { - let mut connection = self.get_connection(); + pub async fn set_nameservers(&mut self, addresses: Vec) -> zbus::fdo::Result<()> { + let mut connection = self.get_connection().await; let ipv4 = connection.ipv4_mut(); addresses .iter() @@ -310,7 +310,7 @@ impl Ipv4 { .collect::, AddrParseError>>() .map(|parsed| ipv4.nameservers = parsed) .map_err(NetworkStateError::from)?; - self.update_connection(connection) + self.update_connection(connection).await } /// Network gateway. @@ -318,8 +318,8 @@ impl Ipv4 { /// An empty string removes the current value. It is not possible to set a gateway if the /// Addresses property is empty. #[dbus_interface(property)] - pub fn gateway(&self) -> String { - let connection = self.get_connection(); + pub async fn gateway(&self) -> String { + let connection = self.get_connection().await; match connection.ipv4().gateway { Some(addr) => addr.to_string(), None => "".to_string(), @@ -327,8 +327,8 @@ impl Ipv4 { } #[dbus_interface(property)] - pub fn set_gateway(&mut self, gateway: String) -> zbus::fdo::Result<()> { - let mut connection = self.get_connection(); + pub async fn set_gateway(&mut self, gateway: String) -> zbus::fdo::Result<()> { + let mut connection = self.get_connection().await; let ipv4 = connection.ipv4_mut(); if gateway.is_empty() { ipv4.gateway = None; @@ -336,7 +336,7 @@ impl Ipv4 { let parsed: Ipv4Addr = gateway.parse().map_err(NetworkStateError::from)?; ipv4.gateway = Some(parsed); } - self.update_connection(connection) + self.update_connection(connection).await } } /// D-Bus interface for wireless settings @@ -360,8 +360,8 @@ impl Wireless { /// Gets the wireless connection. /// /// Beware that it crashes when it is not a wireless connection. - fn get_wireless(&self) -> MappedMutexGuard { - MutexGuard::map(self.connection.lock(), |c| match c { + async fn get_wireless(&self) -> MappedMutexGuard { + MutexGuard::map(self.connection.lock().await, |c| match c { NetworkConnection::Wireless(config) => config, _ => panic!("Not a wireless network. This is most probably a bug."), }) @@ -370,11 +370,11 @@ impl Wireless { /// Updates the connection data in the NetworkSystem. /// /// * `connection`: Updated connection. - fn update_connection( + async fn update_connection<'a>( &self, - connection: MappedMutexGuard, + connection: MappedMutexGuard<'a, NetworkConnection, WirelessConnection>, ) -> zbus::fdo::Result<()> { - let actions = self.actions.lock(); + let actions = self.actions.lock().await; let connection = NetworkConnection::Wireless(connection.clone()); actions.send(Action::UpdateConnection(connection)).unwrap(); Ok(()) @@ -385,16 +385,16 @@ impl Wireless { impl Wireless { /// Network SSID. #[dbus_interface(property, name = "SSID")] - pub fn ssid(&self) -> Vec { - let connection = self.get_wireless(); + pub async fn ssid(&self) -> Vec { + let connection = self.get_wireless().await; connection.wireless.ssid.clone().into() } #[dbus_interface(property, name = "SSID")] - pub fn set_ssid(&mut self, ssid: Vec) -> zbus::fdo::Result<()> { - let mut connection = self.get_wireless(); + pub async fn set_ssid(&mut self, ssid: Vec) -> zbus::fdo::Result<()> { + let mut connection = self.get_wireless().await; connection.wireless.ssid = SSID(ssid); - self.update_connection(connection) + self.update_connection(connection).await } /// Wireless connection mode. @@ -403,22 +403,22 @@ impl Wireless { /// /// See [crate::network::model::WirelessMode]. #[dbus_interface(property)] - pub fn mode(&self) -> String { - let connection = self.get_wireless(); + pub async fn mode(&self) -> String { + let connection = self.get_wireless().await; connection.wireless.mode.to_string() } #[dbus_interface(property)] - pub fn set_mode(&mut self, mode: &str) -> zbus::fdo::Result<()> { - let mut connection = self.get_wireless(); + pub async fn set_mode(&mut self, mode: &str) -> zbus::fdo::Result<()> { + let mut connection = self.get_wireless().await; connection.wireless.mode = mode.try_into()?; - self.update_connection(connection) + self.update_connection(connection).await } /// Password to connect to the wireless network. #[dbus_interface(property)] - pub fn password(&self) -> String { - let connection = self.get_wireless(); + pub async fn password(&self) -> String { + let connection = self.get_wireless().await; connection .wireless .password @@ -427,14 +427,14 @@ impl Wireless { } #[dbus_interface(property)] - pub fn set_password(&mut self, password: String) -> zbus::fdo::Result<()> { - let mut connection = self.get_wireless(); + pub async fn set_password(&mut self, password: String) -> zbus::fdo::Result<()> { + let mut connection = self.get_wireless().await; connection.wireless.password = if password.is_empty() { None } else { Some(password) }; - self.update_connection(connection) + self.update_connection(connection).await } /// Wireless security protocol. @@ -444,18 +444,18 @@ impl Wireless { /// /// See [crate::network::model::SecurityProtocol]. #[dbus_interface(property)] - pub fn security(&self) -> String { - let connection = self.get_wireless(); + pub async fn security(&self) -> String { + let connection = self.get_wireless().await; connection.wireless.security.to_string() } #[dbus_interface(property)] - pub fn set_security(&mut self, security: &str) -> zbus::fdo::Result<()> { - let mut connection = self.get_wireless(); + pub async fn set_security(&mut self, security: &str) -> zbus::fdo::Result<()> { + let mut connection = self.get_wireless().await; connection.wireless.security = security .try_into() .map_err(|_| NetworkStateError::InvalidSecurityProtocol(security.to_string()))?; - self.update_connection(connection)?; + self.update_connection(connection).await?; Ok(()) } } diff --git a/rust/agama-dbus-server/src/network/dbus/tree.rs b/rust/agama-dbus-server/src/network/dbus/tree.rs index 6230e0a8a5..9dea622fa5 100644 --- a/rust/agama-dbus-server/src/network/dbus/tree.rs +++ b/rust/agama-dbus-server/src/network/dbus/tree.rs @@ -1,5 +1,5 @@ use agama_lib::error::ServiceError; -use parking_lot::Mutex; +use futures::lock::Mutex; use zbus::zvariant::{ObjectPath, OwnedObjectPath}; use crate::network::{action::Action, dbus::interfaces, model::*}; @@ -64,7 +64,7 @@ impl Tree { let path = ObjectPath::try_from(path.as_str()).unwrap(); self.add_interface(&path, interfaces::Device::new(dev.clone())) .await?; - let mut objects = self.objects.lock(); + let mut objects = self.objects.lock().await; objects.register_device(&dev.name, path); } @@ -81,7 +81,7 @@ impl Tree { /// /// * `connection`: connection to add. pub async fn add_connection(&self, conn: &mut Connection) -> Result<(), ServiceError> { - let mut objects = self.objects.lock(); + let mut objects = self.objects.lock().await; let (id, path) = objects.register_connection(conn); if id != conn.id() { @@ -114,7 +114,7 @@ impl Tree { /// /// * `id`: connection ID. pub async fn remove_connection(&mut self, id: &str) -> Result<(), ServiceError> { - let mut objects = self.objects.lock(); + let mut objects = self.objects.lock().await; let Some(path) = objects.connection_path(id) else { return Ok(()) }; @@ -142,7 +142,7 @@ impl Tree { /// Clears all the connections from the tree. async fn remove_connections(&self) -> Result<(), ServiceError> { - let mut objects = self.objects.lock(); + let mut objects = self.objects.lock().await; for path in objects.connections.values() { self.remove_connection_on(path.as_str()).await?; } @@ -153,7 +153,7 @@ impl Tree { /// Clears all the devices from the tree. async fn remove_devices(&mut self) -> Result<(), ServiceError> { let object_server = self.connection.object_server(); - let mut objects = self.objects.lock(); + let mut objects = self.objects.lock().await; for path in objects.devices.values() { object_server .remove::(path.as_str())