Bug 1430064 - Use chan_signal to handle signals sent to geckodriver, r=ato
This requires a refactor of the WebDriver main loop so that we are able to send
a message from the geckodriver process in. Hyper doesn't allow us to
exit it's handler loop cleanly so we take the approach of doing the
process cleanup that we need and then simply terminating the
process. That design works OK for geckodriver, but is probably wrong
for other users who may want to be able to shut down the server
without quitting the entire process. So we should make it a priority
to fix this when we upgrade to a newer hyper.
MozReview-Commit-ID: K1Z4teMlL0Z
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -136,16 +136,29 @@ dependencies = [
"lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"peeking_take_while 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
"quote 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)",
"regex 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
"which 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
+name = "bit-set"
+version = "0.4.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "bit-vec 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
+[[package]]
+name = "bit-vec"
+version = "0.4.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
+[[package]]
name = "bitflags"
version = "0.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "bitflags"
version = "1.0.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -211,16 +224,35 @@ dependencies = [
]
[[package]]
name = "cfg-if"
version = "0.1.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
+name = "chan"
+version = "0.1.21"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "rand 0.3.18 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
+[[package]]
+name = "chan-signal"
+version = "0.3.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+dependencies = [
+ "bit-set 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "chan 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "lazy_static 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)",
+ "libc 0.2.33 (registry+https://github.com/rust-lang/crates.io-index)",
+]
+
+[[package]]
name = "chrono"
version = "0.2.25"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"num 0.1.40 (registry+https://github.com/rust-lang/crates.io-index)",
"time 0.1.38 (registry+https://github.com/rust-lang/crates.io-index)",
]
@@ -662,16 +694,18 @@ dependencies = [
"winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
"winapi-build 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "geckodriver"
version = "0.19.1"
dependencies = [
+ "chan 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)",
+ "chan-signal 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
"chrono 0.2.25 (registry+https://github.com/rust-lang/crates.io-index)",
"clap 2.29.0 (registry+https://github.com/rust-lang/crates.io-index)",
"hyper 0.10.13 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
"mozprofile 0.3.0",
"mozrunner 0.5.0",
"mozversion 0.1.2",
@@ -2176,28 +2210,32 @@ dependencies = [
"checksum app_units 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "29069a9b483f7780aebb55dafb360c6225eefdc1f98c8d336a65148fd10c37b1"
"checksum arrayvec 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)" = "2f0ef4a9820019a0c91d918918c93dc71d469f581a49b47ddc1d285d4270bbe2"
"checksum atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fb2dcb6e6d35f20276943cc04bb98e538b348d525a04ac79c10021561d202f21"
"checksum atty 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d912da0db7fa85514874458ca3651fe2cddace8d0b0505571dbdcd41ab490159"
"checksum base64 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "96434f987501f0ed4eb336a411e0631ecd1afa11574fe148587adc4ff96143c9"
"checksum binary-space-partition 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "88ceb0d16c4fd0e42876e298d7d3ce3780dd9ebdcbe4199816a32c77e08597ff"
"checksum bincode 0.9.1 (registry+https://github.com/rust-lang/crates.io-index)" = "9d3fb369af639822830328794eba2501b3479652fcd021b2aeb1ed4984202afd"
"checksum bindgen 0.33.1 (registry+https://github.com/rust-lang/crates.io-index)" = "1657d607dd7a8e10b3181149f60f3b27ea0eac81058c09a1c791b8f6ead91f19"
+"checksum bit-set 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "d9bf6104718e80d7b26a68fdbacff3481cfc05df670821affc7e9cbc1884400c"
+"checksum bit-vec 0.4.4 (registry+https://github.com/rust-lang/crates.io-index)" = "02b4ff8b16e6076c3e14220b39fbc1fabb6737522281a388998046859400895f"
"checksum bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)" = "aad18937a628ec6abcd26d1489012cc0e18c21798210f491af69ded9b881106d"
"checksum bitflags 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "b3c30d3802dfb7281680d6285f2ccdaa8c2d8fee41f93805dba5c4cf50dc23cf"
"checksum bitreader 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "80b13e2ab064ff3aa0bdbf1eff533f9822dc37899821f5f98c67f263eab51707"
"checksum boxfnonce 0.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8380105befe91099e6f69206164072c05bc92427ff6aa8a5171388317346dd75"
"checksum build_const 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "e90dc84f5e62d2ebe7676b83c22d33b6db8bd27340fb6ffbff0a364efa0cb9c9"
"checksum byteorder 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "652805b7e73fada9d85e9a6682a4abd490cb52d96aeecc12e33a0de34dfd0d23"
"checksum bytes 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)" = "d828f97b58cc5de3e40c421d0cf2132d6b2da4ee0e11b8632fa838f0f9333ad6"
"checksum bzip2 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c3eafc42c44e0d827de6b1c131175098fe7fb53b8ce8a47e65cb3ea94688be24"
"checksum bzip2-sys 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)" = "2c5162604199bbb17690ede847eaa6120a3f33d5ab4dcc8e7c25b16d849ae79b"
"checksum cc 1.0.4 (registry+https://github.com/rust-lang/crates.io-index)" = "deaf9ec656256bb25b404c51ef50097207b9cbb29c933d31f92cae5a8a0ffee0"
"checksum cexpr 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "393a5f0088efbe41f9d1fcd062f24e83c278608420e62109feb2c8abee07de7d"
"checksum cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "d4c819a1287eb618df47cc647173c5c4c66ba19d888a6e50d605672aed3140de"
+"checksum chan 0.1.21 (registry+https://github.com/rust-lang/crates.io-index)" = "9af7c487bb99c929ba2715b1a3a7bf45f5062bf5b6eae5d32b292a96c5865172"
+"checksum chan-signal 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "f1f1e11f6e1c14c9e805a87c622cb8fcb636283b3119a2150af390cc6702d7fe"
"checksum chrono 0.2.25 (registry+https://github.com/rust-lang/crates.io-index)" = "9213f7cd7c27e95c2b57c49f0e69b1ea65b27138da84a170133fd21b07659c00"
"checksum clang-sys 0.21.1 (registry+https://github.com/rust-lang/crates.io-index)" = "00048189ee171715296dfe3b2fcfd439563c7bfec0d98d3976ce3402d62c8f07"
"checksum clap 2.29.0 (registry+https://github.com/rust-lang/crates.io-index)" = "110d43e343eb29f4f51c1db31beb879d546db27998577e5715270a54bcf41d3f"
"checksum cmake 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)" = "56d741ea7a69e577f6d06b36b7dff4738f680593dc27a701ffa8506b73ce28bb"
"checksum cookie 0.10.1 (registry+https://github.com/rust-lang/crates.io-index)" = "746858cae4eae40fff37e1998320068df317bc247dc91a67c6cfa053afdc2abb"
"checksum core-foundation 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "286e0b41c3a20da26536c6000a280585d519fd07b3956b43aed8a79e9edce980"
"checksum core-foundation-sys 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "716c271e8613ace48344f723b60b900a93150271e5be206212d052bbc0883efa"
"checksum core-graphics 0.13.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fb0ed45fdc32f9ab426238fba9407dfead7bacd7900c9b4dd3f396f46eafdae3"
--- a/testing/geckodriver/Cargo.toml
+++ b/testing/geckodriver/Cargo.toml
@@ -5,16 +5,18 @@ authors = ["Mozilla <tools-marionette@li
description = "Proxy for using WebDriver clients to interact with Gecko-based browsers."
keywords = ["webdriver", "w3c", "httpd", "mozilla", "firefox"]
repository = "https://hg.mozilla.org/mozilla-central/file/tip/testing/geckodriver"
readme = "README.md"
license = "MPL-2.0"
publish = false
[dependencies]
+chan = "0.1"
+chan-signal = "0.3"
chrono = "^0.2"
clap = { version = "^2.19", default-features = false, features = ["suggestions", "wrap_help"] }
hyper = "0.10"
lazy_static = "1.0"
log = { version = "0.4", features = ["std"] }
mozprofile = { path = "../mozbase/rust/mozprofile" }
mozrunner = { path = "../mozbase/rust/mozrunner" }
mozversion = { path = "../mozbase/rust/mozversion" }
--- a/testing/geckodriver/src/main.rs
+++ b/testing/geckodriver/src/main.rs
@@ -1,8 +1,10 @@
+extern crate chan;
+extern crate chan_signal;
extern crate chrono;
#[macro_use]
extern crate clap;
#[macro_use]
extern crate lazy_static;
extern crate hyper;
extern crate mozprofile;
extern crate mozrunner;
@@ -11,23 +13,26 @@ extern crate regex;
extern crate rustc_serialize;
extern crate uuid;
extern crate zip;
extern crate webdriver;
#[macro_use]
extern crate log;
+use clap::{App, Arg};
+use chan::Receiver;
+use chan_signal::Signal;
use std::fmt;
use std::io::Write;
use std::net::{IpAddr, SocketAddr};
use std::path::PathBuf;
use std::str::FromStr;
-
-use clap::{App, Arg};
+use std::sync::mpsc::Sender;
+use std::thread;
macro_rules! try_opt {
($expr:expr, $err_type:expr, $err_msg:expr) => ({
match $expr {
Some(x) => x,
None => return Err(WebDriverError::new($err_type, $err_msg))
}
})
@@ -185,30 +190,54 @@ fn run() -> ProgramResult {
if let Some(ref level) = log_level {
logging::init_with_level(level.clone()).unwrap();
} else {
logging::init().unwrap();
}
info!("geckodriver {}", BuildInfo);
+ let signal = chan_signal::notify(&[Signal::INT, Signal::TERM]);
+
let settings = MarionetteSettings {
port: marionette_port,
binary,
connect_existing: matches.is_present("connect_existing"),
jsdebugger: matches.is_present("jsdebugger"),
};
let handler = MarionetteHandler::new(settings);
- let listening = webdriver::server::start(addr, handler, &extension_routes()[..])
+
+ let server = webdriver::server::WebDriverServer::new();
+
+ let builder = thread::Builder::new().name("signal handler".to_string());
+ let chan = server.send_chan();
+ builder.spawn(move || {
+ signal_notify(signal, chan);
+ }).unwrap();
+
+ let listening = server.start(addr, handler, &extension_routes()[..])
.map_err(|err| (ExitCode::Unavailable, err.to_string()))?;
+
info!("Listening on {}", listening.socket);
Ok(())
}
+fn signal_notify<U>(signal: Receiver<Signal>,
+ send_chan: Sender<webdriver::server::DispatchMessage<U>>)
+ where U: 'static + webdriver::httpapi::WebDriverExtensionRoute
+{
+
+ // Blocks until this process is sent an INT or TERM signal.
+ signal.recv().unwrap();
+
+ debug!("Got a signal, quitting");
+ send_chan.send(webdriver::server::DispatchMessage::Quit).unwrap();
+}
+
fn main() {
let exit_code = match run() {
Ok(_) => ExitCode::Ok,
Err((exit_code, reason)) => {
error!("{}", reason);
exit_code
}
};
--- a/testing/geckodriver/src/marionette.rs
+++ b/testing/geckodriver/src/marionette.rs
@@ -599,17 +599,16 @@ impl WebDriverHandler<GeckoExtensionRout
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");
}
}
-
self.connection = Mutex::new(None);
self.browser = None;
}
}
pub struct MarionetteSession {
pub session_id: String,
protocol: Option<String>,
--- a/testing/webdriver/src/server.rs
+++ b/testing/webdriver/src/server.rs
@@ -1,29 +1,29 @@
+use hyper::Result;
+use hyper::header::{ContentType, CacheControl, CacheDirective};
+use hyper::method::Method;
+use hyper::mime::{Mime, TopLevel, SubLevel, Attr, Value};
+use hyper::server::{Handler, Listening, Request, Response, Server};
+use hyper::status::StatusCode;
+use hyper::uri::RequestUri::AbsolutePath;
use std::io::Read;
use std::marker::PhantomData;
use std::net::SocketAddr;
+use std::process;
+use std::sync::Mutex;
use std::sync::mpsc::{channel, Receiver, Sender};
-use std::sync::Mutex;
use std::thread;
-use hyper::header::{ContentType, CacheControl, CacheDirective};
-use hyper::mime::{Mime, TopLevel, SubLevel, Attr, Value};
-use hyper::method::Method;
-use hyper::Result;
-use hyper::server::{Handler, Listening, Request, Response, Server};
-use hyper::status::StatusCode;
-use hyper::uri::RequestUri::AbsolutePath;
-
use command::{WebDriverMessage, WebDriverCommand};
use error::{WebDriverResult, WebDriverError, ErrorStatus};
use httpapi::{WebDriverHttpApi, WebDriverExtensionRoute, VoidWebDriverExtensionRoute};
use response::{CloseWindowResponse, WebDriverResponse};
-enum DispatchMessage<U: WebDriverExtensionRoute> {
+pub enum DispatchMessage<U: WebDriverExtensionRoute> {
HandleWebDriver(WebDriverMessage<U>, Sender<WebDriverResult<WebDriverResponse>>),
Quit
}
#[derive(Clone, Debug, PartialEq)]
pub struct Session {
pub id: String
}
@@ -55,17 +55,18 @@ impl<T: WebDriverHandler<U>, U: WebDrive
handler: handler,
session: None,
extension_type: PhantomData,
}
}
fn run(&mut self, msg_chan: Receiver<DispatchMessage<U>>) {
loop {
- match msg_chan.recv() {
+ let msg = msg_chan.recv();
+ match msg {
Ok(DispatchMessage::HandleWebDriver(msg, resp_chan)) => {
let resp = match self.check_session(&msg) {
Ok(_) => self.handler.handle_command(&self.session, msg),
Err(e) => Err(e),
};
match resp {
Ok(WebDriverResponse::NewSession(ref new_session)) => {
@@ -81,17 +82,22 @@ impl<T: WebDriverHandler<U>, U: WebDrive
Err(ref x) if x.delete_session => self.delete_session(),
_ => {}
}
if resp_chan.send(resp).is_err() {
error!("Sending response to the main thread failed");
};
}
- Ok(DispatchMessage::Quit) => break,
+ Ok(DispatchMessage::Quit) => {
+ debug!("WebDriver asked to quit");
+ self.delete_session();
+ // Terminating hyper doesn't work so just directly exit here
+ process::exit(0);
+ },
Err(_) => panic!("Error receiving message in handler"),
}
}
}
fn delete_session(&mut self) {
debug!("Deleting session");
self.handler.delete_session(&self.session);
@@ -234,30 +240,50 @@ impl<U: WebDriverExtensionRoute> Handler
res.send(&resp_body.as_bytes()).unwrap();
}
_ => {}
}
}
}
-pub fn start<T, U>(address: SocketAddr,
- handler: T,
- extension_routes: &[(Method, &str, U)])
- -> Result<Listening>
- where T: 'static + WebDriverHandler<U>,
- U: 'static + WebDriverExtensionRoute
+pub struct WebDriverServer<U>
+ where U: 'static + WebDriverExtensionRoute
{
- let (msg_send, msg_recv) = channel();
+ send_chan: Sender<DispatchMessage<U>>,
+ recv_chan: Receiver<DispatchMessage<U>>,
+}
+
+impl <U> WebDriverServer<U>
+where U: 'static + WebDriverExtensionRoute {
+ pub fn new() -> Self {
+ let (send_chan, recv_chan) = channel();
+ WebDriverServer {
+ send_chan,
+ recv_chan
+ }
+ }
- let api = WebDriverHttpApi::new(extension_routes);
- let http_handler = HttpHandler::new(api, msg_send);
- let mut server = try!(Server::http(address));
- server.keep_alive(None);
+ pub fn start<T>(self,
+ address: SocketAddr,
+ handler: T,
+ extension_routes: &[(Method, &str, U)])
+ -> Result<Listening>
+ where T: 'static + WebDriverHandler<U>,
+ {
+ let api = WebDriverHttpApi::new(extension_routes);
+ let http_handler = HttpHandler::new(api, self.send_chan());
+ let mut server = try!(Server::http(address));
+ server.keep_alive(None);
- let builder = thread::Builder::new().name("webdriver dispatcher".to_string());
- try!(builder.spawn(move || {
- let mut dispatcher = Dispatcher::new(handler);
- dispatcher.run(msg_recv);
- }));
+ let builder = thread::Builder::new().name("webdriver dispatcher".to_string());
+ try!(builder.spawn(move || {
+ let mut dispatcher = Dispatcher::new(handler);
+ dispatcher.run(self.recv_chan);
+ }));
- server.handle(http_handler)
+ server.handle(http_handler)
+ }
+
+ pub fn send_chan(&self) -> Sender<DispatchMessage<U>> {
+ self.send_chan.clone()
+ }
}