Fix a few TODO comments

mullvad-daemon/Cargo.toml

@@ -20,7 +20,6 @@ futures = "0.3"
 once_cell = "1.13"
 libc = "0.2"
 log = { workspace = true }
-parking_lot = "0.12.0"
 regex = "1.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"

mullvad-daemon/src/management_interface.rs

@@ -23,12 +23,11 @@ use mullvad_types::{
     version,
     wireguard::{RotationInterval, RotationIntervalError},
 };
-use parking_lot::RwLock;
 #[cfg(windows)]
 use std::path::PathBuf;
 use std::{
     convert::{TryFrom, TryInto},
-    sync::Arc,
+    sync::{Arc, Mutex},
     time::Duration,
 };
 use talpid_types::ErrorExt;
@@ -44,7 +43,7 @@ pub enum Error {
 
 struct ManagementServiceImpl {
     daemon_tx: DaemonCommandSender,
-    subscriptions: Arc<RwLock<Vec<EventsListenerSender>>>,
+    subscriptions: Arc<Mutex<Vec<EventsListenerSender>>>,
 }
 
 pub type ServiceResult<T> = std::result::Result<Response<T>, Status>;
@@ -102,7 +101,7 @@ impl ManagementService for ManagementServiceImpl {
     async fn events_listen(&self, _: Request<()>) -> ServiceResult<Self::EventsListenStream> {
         let (tx, rx) = tokio::sync::mpsc::unbounded_channel();
 
-        let mut subscriptions = self.subscriptions.write();
+        let mut subscriptions = self.subscriptions.lock().unwrap();
         subscriptions.push(tx);
 
         Ok(Response::new(UnboundedReceiverStream::new(rx)))
@@ -881,7 +880,7 @@ impl ManagementInterfaceServer {
     pub fn start(
         tunnel_tx: DaemonCommandSender,
     ) -> Result<(String, ManagementInterfaceEventBroadcaster), Error> {
-        let subscriptions = Arc::<RwLock<Vec<EventsListenerSender>>>::default();
+        let subscriptions = Arc::<Mutex<Vec<EventsListenerSender>>>::default();
 
         let socket_path = mullvad_paths::get_rpc_socket_path()
             .to_string_lossy()
@@ -917,7 +916,7 @@ impl ManagementInterfaceServer {
 /// A handle that allows broadcasting messages to all subscribers of the management interface.
 #[derive(Clone)]
 pub struct ManagementInterfaceEventBroadcaster {
-    subscriptions: Arc<RwLock<Vec<EventsListenerSender>>>,
+    subscriptions: Arc<Mutex<Vec<EventsListenerSender>>>,
     _close_handle: mpsc::Sender<()>,
 }
 
@@ -981,8 +980,7 @@ impl EventListener for ManagementInterfaceEventBroadcaster {
 
 impl ManagementInterfaceEventBroadcaster {
     fn notify(&self, value: types::DaemonEvent) {
-        let mut subscriptions = self.subscriptions.write();
-        // TODO: using write-lock everywhere. use a mutex instead?
+        let mut subscriptions = self.subscriptions.lock().unwrap();
         subscriptions.retain(|tx| tx.send(Ok(value.clone())).is_ok());
     }
 }

mullvad-daemon/src/migrations/mod.rs

@@ -369,8 +369,6 @@ mod windows {
         pub fn owner(&self) -> Option<&SID> {
             unsafe { (self.owner as *const SID).as_ref() }
         }
-
-        // TODO: Can be expanded with `group()`, `dacl()`, and `sacl()`.
     }
 
     impl Drop for SecurityInformation {

talpid-core/src/dns/windows/iphlpapi.rs

@@ -81,7 +81,8 @@ impl IphlpApi {
         }
 
         // This function is loaded at runtime since it may be unavailable. See the module-level docs.
-        // TODO: `windows_sys` can be used directly when support for Windows 10, 2004, is dropped.
+        // TODO: `windows_sys` can be used directly when support for versions older than Windows 10,
+        // 2004, is dropped.
         let set_interface_dns_settings =
             unsafe { GetProcAddress(module, s!("SetInterfaceDnsSettings")) };
         let set_interface_dns_settings = set_interface_dns_settings.ok_or_else(|| {

talpid-core/src/split_tunnel/linux.rs

@@ -125,12 +125,8 @@ impl PidManager {
     pub fn remove(&self, pid: i32) -> Result<(), Error> {
         // FIXME: We remove PIDs from our cgroup here by adding
         //        them to the parent cgroup. This seems wrong.
-        let exclusions_path = self.net_cls_path.join("cgroup.procs");
-
-        let mut file = fs::OpenOptions::new()
-            .write(true)
-            .create(true)
-            .open(exclusions_path)
+        let mut file = self
+            .open_parent_cgroup_handle()
             .map_err(Error::RemoveCGroupPid)?;
 
         file.write_all(pid.to_string().as_bytes())
@@ -160,13 +156,23 @@ impl PidManager {
 
     /// Removes all PIDs from the Cgroup.
     pub fn clear(&self) -> Result<(), Error> {
-        // TODO: reuse file handle
         let pids = self.list()?;
 
+        let mut file = self
+            .open_parent_cgroup_handle()
+            .map_err(Error::RemoveCGroupPid)?;
         for pid in pids {
-            self.remove(pid)?;
+            file.write_all(pid.to_string().as_bytes())
+                .map_err(Error::RemoveCGroupPid)?;
         }
 
         Ok(())
     }
+
+    fn open_parent_cgroup_handle(&self) -> io::Result<fs::File> {
+        fs::OpenOptions::new()
+            .write(true)
+            .create(true)
+            .open(self.net_cls_path.join("cgroup.procs"))
+    }
 }