Skip to content

Commit

Permalink
Merge branch 'cleanup-todos'
Browse files Browse the repository at this point in the history
  • Loading branch information
dlon committed Sep 11, 2023
2 parents 64d0b37 + ee0e670 commit ba70363
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 21 deletions.
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion mullvad-daemon/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 6 additions & 8 deletions mullvad-daemon/src/management_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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>;
Expand Down Expand Up @@ -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)))
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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<()>,
}

Expand Down Expand Up @@ -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());
}
}
Expand Down
2 changes: 0 additions & 2 deletions mullvad-daemon/src/migrations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion talpid-core/src/dns/windows/iphlpapi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(|| {
Expand Down
22 changes: 14 additions & 8 deletions talpid-core/src/split_tunnel/linux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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"))
}
}

0 comments on commit ba70363

Please sign in to comment.