Skip to content

Commit

Permalink
Remove dependency on duct
Browse files Browse the repository at this point in the history
Remove the dependency on `duct` from `talpid-openvpn`, since we can use
`tokio` to spawn processes instead.
  • Loading branch information
MarkusPettersson98 committed Oct 9, 2023
1 parent 014c00e commit 62e999a
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 18 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 talpid-openvpn/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
9 changes: 6 additions & 3 deletions talpid-openvpn/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ impl<C: OpenVpnBuilder + Send + 'static> OpenVpnMonitor<C> {

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();
Expand Down Expand Up @@ -799,7 +799,7 @@ impl OpenVpnBuilder for OpenVpnCommand {
}

fn start(&self) -> io::Result<OpenVpnProcHandle> {
OpenVpnProcHandle::new(self.build())
OpenVpnProcHandle::new(&mut self.build())
}

#[cfg(target_os = "linux")]
Expand All @@ -811,7 +811,10 @@ impl OpenVpnBuilder for OpenVpnCommand {

impl ProcessHandle for OpenVpnProcHandle {
fn wait(&self) -> io::Result<ExitStatus> {
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<()> {
Expand Down
39 changes: 26 additions & 13 deletions talpid-openvpn/src/process/openvpn.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use duct;

use super::stoppable_process::StoppableProcess;
use os_pipe::{pipe, PipeWriter};
use parking_lot::Mutex;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<tokio::sync::Mutex<tokio::process::Child>>,
/// 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
Expand All @@ -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<Self> {
pub fn new(mut cmd: &mut tokio::process::Command) -> io::Result<Self> {
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)),
})
}
Expand All @@ -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<bool> {
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),
Expand Down

0 comments on commit 62e999a

Please sign in to comment.