From 62e999ace9dbcdd9b5634977778bc46e31bd1c09 Mon Sep 17 00:00:00 2001 From: Markus Pettersson Date: Mon, 9 Oct 2023 09:05:50 +0200 Subject: [PATCH] Remove dependency on `duct` Remove the dependency on `duct` from `talpid-openvpn`, since we can use `tokio` to spawn processes instead. --- Cargo.lock | 1 - talpid-openvpn/Cargo.toml | 1 - talpid-openvpn/src/lib.rs | 9 ++++--- talpid-openvpn/src/process/openvpn.rs | 39 ++++++++++++++++++--------- 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 02a94a8c4b8c..095760da3e75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3486,7 +3486,6 @@ name = "talpid-openvpn" version = "0.0.0" dependencies = [ "async-trait", - "duct", "err-derive", "futures", "log", diff --git a/talpid-openvpn/Cargo.toml b/talpid-openvpn/Cargo.toml index 294012a373e9..8106c4816d1f 100644 --- a/talpid-openvpn/Cargo.toml +++ b/talpid-openvpn/Cargo.toml @@ -11,7 +11,6 @@ publish.workspace = true [dependencies] async-trait = "0.1" -duct = "0.13" err-derive = { workspace = true } futures = "0.3.15" once_cell = { workspace = true } diff --git a/talpid-openvpn/src/lib.rs b/talpid-openvpn/src/lib.rs index 6d2989160edb..8892c56262f3 100644 --- a/talpid-openvpn/src/lib.rs +++ b/talpid-openvpn/src/lib.rs @@ -590,7 +590,7 @@ impl OpenVpnMonitor { let event_server_abort_tx = self.event_server_abort_tx.clone(); - thread::spawn(move || { + self.runtime.spawn_blocking(move || { let result = child.wait(); let closed = closed_handle.load(Ordering::SeqCst); child_tx.send(WaitResult::Child(result, closed)).unwrap(); @@ -799,7 +799,7 @@ impl OpenVpnBuilder for OpenVpnCommand { } fn start(&self) -> io::Result { - OpenVpnProcHandle::new(self.build()) + OpenVpnProcHandle::new(&mut self.build()) } #[cfg(target_os = "linux")] @@ -811,7 +811,10 @@ impl OpenVpnBuilder for OpenVpnCommand { impl ProcessHandle for OpenVpnProcHandle { fn wait(&self) -> io::Result { - self.inner.wait().map(|output| output.status) + let handle = tokio::runtime::Handle::current(); + let inner_handle = self.inner.clone(); + let wait_exit = async move { inner_handle.lock().await.wait().await }; + handle.block_on(wait_exit) } fn kill(&self) -> io::Result<()> { diff --git a/talpid-openvpn/src/process/openvpn.rs b/talpid-openvpn/src/process/openvpn.rs index 0fbb292e2eac..9d5747f27b12 100644 --- a/talpid-openvpn/src/process/openvpn.rs +++ b/talpid-openvpn/src/process/openvpn.rs @@ -1,5 +1,3 @@ -use duct; - use super::stoppable_process::StoppableProcess; use os_pipe::{pipe, PipeWriter}; use parking_lot::Mutex; @@ -190,9 +188,11 @@ impl OpenVpnCommand { } /// Build a runnable expression from the current state of the command. - pub fn build(&self) -> duct::Expression { + pub fn build(&self) -> tokio::process::Command { log::debug!("Building expression: {}", &self); - duct::cmd(&self.openvpn_bin, self.get_arguments()).unchecked() + let mut handle = tokio::process::Command::new(&self.openvpn_bin); + handle.args(self.get_arguments()); + handle } /// Returns all arguments that the subprocess would be spawned with. @@ -365,8 +365,11 @@ impl fmt::Display for OpenVpnCommand { /// Handle to a running OpenVPN process. pub struct OpenVpnProcHandle { - /// Duct handle - pub inner: duct::Handle, + /// Handle to the child process running OpenVPN. + /// + /// This handle is acquired by calling [`OpenVpnCommand::build`] (or + /// [`tokio::process::Command::spawn`]). + pub inner: std::sync::Arc>, /// Pipe handle to stdin of the OpenVPN process. Our custom fork of OpenVPN /// has been changed so that it exits cleanly when stdin is closed. This is a hack /// solution to cleanly shut OpenVPN down without using the @@ -377,22 +380,22 @@ pub struct OpenVpnProcHandle { impl OpenVpnProcHandle { /// Configures the expression to run OpenVPN in a way compatible with this handle /// and spawns it. Returns the handle. - pub fn new(mut cmd: duct::Expression) -> io::Result { + pub fn new(mut cmd: &mut tokio::process::Command) -> io::Result { use std::io::IsTerminal; if !std::io::stdout().is_terminal() { - cmd = cmd.stdout_null(); + cmd = cmd.stdout(std::process::Stdio::null()) } if !std::io::stderr().is_terminal() { - cmd = cmd.stderr_null(); + cmd = cmd.stderr(std::process::Stdio::null()) } let (reader, writer) = pipe()?; - let proc_handle = cmd.stdin_file(reader).start()?; + let proc_handle = cmd.stdin(reader).spawn()?; Ok(Self { - inner: proc_handle, + inner: std::sync::Arc::new(tokio::sync::Mutex::new(proc_handle)), stdin: Mutex::new(Some(writer)), }) } @@ -409,13 +412,23 @@ impl StoppableProcess for OpenVpnProcHandle { fn kill(&self) -> io::Result<()> { log::warn!("Killing OpenVPN process"); - self.inner.kill()?; + let rt = tokio::runtime::Builder::new_current_thread() + .build() + .unwrap(); + let inner_handle = self.inner.clone(); + let kill = async move { inner_handle.lock().await.kill().await }; + rt.block_on(kill)?; log::debug!("OpenVPN forcefully killed"); Ok(()) } fn has_stopped(&self) -> io::Result { - match self.inner.try_wait() { + let rt = tokio::runtime::Builder::new_current_thread() + .build() + .unwrap(); + let inner_handle = self.inner.clone(); + let exit_status = rt.block_on(async move { inner_handle.lock().await.try_wait() }); + match exit_status { Ok(None) => Ok(false), Ok(Some(_)) => Ok(true), Err(e) => Err(e),