This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8449

rust-chg: do not terminate tokio runtime until pager exits
ClosedPublic

Authored by yuja on Apr 16 2020, 8:30 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG4b0185841058: rust-chg: do not terminate tokio runtime until pager exits
Summary

We no longer need to spawn a task just to keep the pager handle. The pager
handle can be held by ChgUiHandler since the handler itself is not consumed
and recreated across async calls.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

yuja created this revision.Apr 16 2020, 8:30 AM
Alphare accepted this revision.Apr 17 2020, 8:32 AM
yuja added a commit: rHG4b0185841058: rust-chg: do not terminate tokio runtime until pager exits.Apr 23 2020, 2:03 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG4b0185841058: rust-chg: do not terminate tokio runtime until pager exits (authored by yuja). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/chg/src/main.rs (1 line)
Mrust/chg/src/uihandler.rs (18 lines)

Diff 21193

rust/chg/src/main.rs

client.set_umask(umask).await?; client.set_umask(umask).await?;
let pid = client.server_spec().process_id.unwrap(); let pid = client.server_spec().process_id.unwrap();
let pgid = client.server_spec().process_group_id; let pgid = client.server_spec().process_group_id;
procutil::setup_signal_handler_once(pid, pgid)?; procutil::setup_signal_handler_once(pid, pgid)?;
let code = client let code = client
.run_command_chg(&mut handler, env::args_os().skip(1)) .run_command_chg(&mut handler, env::args_os().skip(1))
.await?; .await?;
procutil::restore_signal_handler_once()?; procutil::restore_signal_handler_once()?;
handler.wait_pager().await?;
Ok(code) Ok(code)
} }

rust/chg/src/uihandler.rs

// Copyright 2018 Yuya Nishihara <yuya@tcha.org> // Copyright 2018 Yuya Nishihara <yuya@tcha.org>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
use async_trait::async_trait; use async_trait::async_trait;
use std::io; use std::io;
use std::os::unix::io::AsRawFd; use std::os::unix::io::AsRawFd;
use std::os::unix::process::ExitStatusExt; use std::os::unix::process::ExitStatusExt;
use std::process::Stdio; use std::process::Stdio;
use tokio; use tokio;
use tokio::process::{ChildStdin, Command}; use tokio::process::{Child, ChildStdin, Command};
use crate::message::CommandSpec; use crate::message::CommandSpec;
use crate::procutil; use crate::procutil;
/// Callback to process shell command requests received from server. /// Callback to process shell command requests received from server.
#[async_trait] #[async_trait]
pub trait SystemHandler { pub trait SystemHandler {
type PagerStdin: AsRawFd; type PagerStdin: AsRawFd;
/// Handles pager command request. /// Handles pager command request.
/// ///
/// Returns the pipe to be attached to the server if the pager is spawned. /// Returns the pipe to be attached to the server if the pager is spawned.
async fn spawn_pager(&mut self, spec: &CommandSpec) -> io::Result<Self::PagerStdin>; async fn spawn_pager(&mut self, spec: &CommandSpec) -> io::Result<Self::PagerStdin>;
/// Handles system command request. /// Handles system command request.
/// ///
/// Returns command exit code (positive) or signal number (negative). /// Returns command exit code (positive) or signal number (negative).
async fn run_system(&mut self, spec: &CommandSpec) -> io::Result<i32>; async fn run_system(&mut self, spec: &CommandSpec) -> io::Result<i32>;
} }
/// Default cHg implementation to process requests received from server. /// Default cHg implementation to process requests received from server.
pub struct ChgUiHandler {} pub struct ChgUiHandler {
pager: Option<Child>,
}
impl ChgUiHandler { impl ChgUiHandler {
pub fn new() -> ChgUiHandler { pub fn new() -> ChgUiHandler {
ChgUiHandler {} ChgUiHandler { pager: None }
}
/// Waits until the pager process exits.
pub async fn wait_pager(&mut self) -> io::Result<()> {
if let Some(p) = self.pager.take() {
p.await?;
}
Ok(())
} }
} }
#[async_trait] #[async_trait]
impl SystemHandler for ChgUiHandler { impl SystemHandler for ChgUiHandler {
type PagerStdin = ChildStdin; type PagerStdin = ChildStdin;
async fn spawn_pager(&mut self, spec: &CommandSpec) -> io::Result<Self::PagerStdin> { async fn spawn_pager(&mut self, spec: &CommandSpec) -> io::Result<Self::PagerStdin> {
let mut pager = new_shell_command(&spec).stdin(Stdio::piped()).spawn()?; let mut pager = new_shell_command(&spec).stdin(Stdio::piped()).spawn()?;
let pin = pager.stdin.take().unwrap(); let pin = pager.stdin.take().unwrap();
procutil::set_blocking_fd(pin.as_raw_fd())?; procutil::set_blocking_fd(pin.as_raw_fd())?;
// TODO: if pager exits, notify the server with SIGPIPE immediately. // TODO: if pager exits, notify the server with SIGPIPE immediately.
// otherwise the server won't get SIGPIPE if it does not write // otherwise the server won't get SIGPIPE if it does not write
// anything. (issue5278) // anything. (issue5278)
// kill(peerpid, SIGPIPE); // kill(peerpid, SIGPIPE);
tokio::spawn(async { pager.await }); // just ignore errors self.pager = Some(pager);
Ok(pin) Ok(pin)
} }
async fn run_system(&mut self, spec: &CommandSpec) -> io::Result<i32> { async fn run_system(&mut self, spec: &CommandSpec) -> io::Result<i32> {
let status = new_shell_command(&spec).spawn()?.await?; let status = new_shell_command(&spec).spawn()?.await?;
// TODO: unindent // TODO: unindent
{ {
{ {