Bug 1448900 - Avoid killing exited process. r?whimboo
std::process::Child::kill() will return Err if the process has
already exited. The assumption in
bug 1443853 was that calling
::kill() would consistently return the std::process::ExitStatus
was the process already dead.
This patches the regression from
bug 1443853 by employing
Child::try_wait() in a loop. When the process gives some exit status,
this is return directly without relying on Child::kill() as before.
If the process has not exited and the timeout has elapsed, we kill
the process and return its return value. If the process has not
exited but the timeout duration has not elapsed, we wait 100 ms as before.
MozReview-Commit-ID: 4VENbrKtcEh
--- a/testing/geckodriver/src/marionette.rs
+++ b/testing/geckodriver/src/marionette.rs
@@ -595,18 +595,19 @@ impl WebDriverHandler<GeckoExtensionRout
if let Some(conn) = connection.as_mut() {
conn.close();
}
}
if let Some(ref mut runner) = self.browser {
// TODO(https://bugzil.la/1443922):
// Use toolkit.asyncshutdown.crash_timout pref
- if runner.wait(time::Duration::from_secs(70)).is_err() {
- error!("Failed to stop browser process");
+ match runner.wait(time::Duration::from_secs(70)) {
+ Ok(x) => debug!("Browser process stopped with exit status {}", x),
+ Err(e) => error!("Failed to stop browser process: {}", e),
}
}
self.connection = Mutex::new(None);
self.browser = None;
}
}
--- a/testing/mozbase/rust/mozrunner/src/runner.rs
+++ b/testing/mozbase/rust/mozrunner/src/runner.rs
@@ -132,31 +132,39 @@ pub struct FirefoxProcess {
}
impl RunnerProcess for FirefoxProcess {
fn try_wait(&mut self) -> io::Result<Option<process::ExitStatus>> {
self.process.try_wait()
}
fn wait(&mut self, timeout: time::Duration) -> io::Result<process::ExitStatus> {
- let now = time::Instant::now();
- while self.running() {
- if now.elapsed() >= timeout {
- break;
+ let start = time::Instant::now();
+ loop {
+ match self.try_wait() {
+ // child has already exited, reap its exit code
+ Ok(Some(status)) => return Ok(status),
+
+ // child still running and timeout elapsed, kill it
+ Ok(None) if start.elapsed() >= timeout => return self.kill(),
+
+ // child still running, let's give it more time
+ Ok(None) => thread::sleep(time::Duration::from_millis(100)),
+
+ Err(e) => return Err(e),
}
- thread::sleep(time::Duration::from_millis(100));
}
- self.kill()
}
fn running(&mut self) -> bool {
self.try_wait().unwrap().is_none()
}
fn kill(&mut self) -> io::Result<process::ExitStatus> {
+ debug!("Killing process {}", self.process.id());
self.process.kill()?;
self.process.wait()
}
}
#[derive(Debug)]
pub struct FirefoxRunner {
binary: PathBuf,