From 5810658d6e3609e7c4140caf395a6fe34838cabb Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 22 Sep 2020 13:28:21 -0700 Subject: [PATCH 01/50] refactor: actor-based dfx start checkpoint - e2e tests, e2e ic ref tests pass --- src/dfx/Cargo.toml | 1 + src/dfx/src/actors.rs | 1 + src/dfx/src/actors/replica.rs | 23 ++- .../actors/replica_webserver_coordinator.rs | 108 +++++++++++++ src/dfx/src/commands/start.rs | 149 ++++++++++++++++++ src/dfx/src/lib/proxy.rs | 9 +- src/dfx/src/lib/webserver.rs | 27 ++-- 7 files changed, 303 insertions(+), 15 deletions(-) create mode 100644 src/dfx/src/actors/replica_webserver_coordinator.rs diff --git a/src/dfx/Cargo.toml b/src/dfx/Cargo.toml index 2315760ccd..adf8ae08e1 100644 --- a/src/dfx/Cargo.toml +++ b/src/dfx/Cargo.toml @@ -27,6 +27,7 @@ clap = "2.33.0" console = "0.7.7" crossbeam = "0.7.3" delay = "0.2.0" +#ctrlc = { version = "3.1.6", features = [ "termination" ] } dialoguer = "0.6.2" erased-serde = "0.3.10" flate2 = "1.0.11" diff --git a/src/dfx/src/actors.rs b/src/dfx/src/actors.rs index 94841653bf..37a554a267 100644 --- a/src/dfx/src/actors.rs +++ b/src/dfx/src/actors.rs @@ -1,2 +1,3 @@ pub mod replica; +pub mod replica_webserver_coordinator; pub mod signal_watcher; diff --git a/src/dfx/src/actors/replica.rs b/src/dfx/src/actors/replica.rs index 4f5d8dec88..a1ee8d227c 100644 --- a/src/dfx/src/actors/replica.rs +++ b/src/dfx/src/actors/replica.rs @@ -90,10 +90,12 @@ impl Replica { .timeout(Duration::from_secs(30)) .build(); + println!("wait_for_port_file({})", file_path.to_string_lossy()); waiter.start(); loop { if let Ok(content) = std::fs::read_to_string(file_path) { if let Ok(port) = content.parse::() { + println!(" - got port {}", port); return Ok(port); } } @@ -127,6 +129,7 @@ impl Replica { addr, receiver, )?; + info!(self.logger, "started replica thread"); self.thread_join = Some(handle); self.stop_sender = Some(sender); @@ -134,6 +137,7 @@ impl Replica { } fn send_ready_signal(&self, port: u16) { + info!(self.logger, "send_ready_signal(port={})", port); for sub in &self.ready_subscribers { let _ = sub.do_send(signals::outbound::ReplicaReadySignal { port }); } @@ -146,6 +150,7 @@ impl Actor for Replica { fn started(&mut self, ctx: &mut Self::Context) { self.start_replica(ctx.address()) .expect("Could not start the replica"); + info!(self.logger, "Replica Actor started..."); } fn stopping(&mut self, _ctx: &mut Self::Context) -> Running { @@ -182,6 +187,10 @@ impl Handler for Replica { type Result = (); fn handle(&mut self, msg: ReplicaRestarted, _ctx: &mut Self::Context) -> Self::Result { + info!( + self.logger, + "Replica::Handler(ReplicaRestarted port={})", msg.port + ); self.port = Some(msg.port); self.send_ready_signal(msg.port); } @@ -238,12 +247,20 @@ fn replica_start_thread( cmd.args(&[ "--replica-path", replica_path.to_str().unwrap_or_default(), - "--http-port", - &port.unwrap_or_default().to_string(), "--state-dir", config.state_manager.state_root.to_str().unwrap_or_default(), "--require-valid-signatures", ]); + if let Some(port) = port { + cmd.args(&["--http-port", &port.to_string()]); + } + if let Some(write_port_to) = &write_port_to { + cmd.args(&[ + "--http-port-file", + &write_port_to.to_string_lossy().to_string(), + ]); + } + cmd.stdout(std::process::Stdio::inherit()); cmd.stderr(std::process::Stdio::inherit()); @@ -259,8 +276,10 @@ fn replica_start_thread( let port = port.unwrap_or_else(|| { Replica::wait_for_port_file(write_port_to.as_ref().unwrap()).unwrap() }); + println!("sending ReplicaRestarted"); addr.do_send(signals::ReplicaRestarted { port }); + println!("wait_for_child_or_receiver..."); // This waits for the child to stop, or the receiver to receive a message. // We don't restart the replica if done = true. match wait_for_child_or_receiver(&mut child, &receiver) { diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs new file mode 100644 index 0000000000..1d839fcd18 --- /dev/null +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -0,0 +1,108 @@ +use crate::actors::replica::signals::outbound::ReplicaReadySignal; +use crate::actors::replica::signals::PortReadySubscribe; +use crate::actors::replica::Replica; +use crate::lib::error::DfxResult; +use crate::lib::proxy::ProxyConfig; +use crate::lib::webserver::run_webserver; +use actix::fut::wrap_future; +use actix::{Actor, Addr, AsyncContext, Context, Handler}; +use actix_server::Server; +use slog::{info, Logger}; + +pub struct Config { + pub replica_addr: Addr, + pub logger: Option, + pub proxy_config: ProxyConfig, +} + +pub struct ReplicaWebserverCoordinator { + logger: Logger, + config: Config, + server: Option, +} + +impl ReplicaWebserverCoordinator { + pub fn new(config: Config) -> Self { + let logger = + (config.logger.clone()).unwrap_or_else(|| Logger::root(slog::Discard, slog::o!())); + ReplicaWebserverCoordinator { + config, + logger, + server: None, + } + } + + fn start_server(&self, port: u16) -> DfxResult> { + let proxy_config = &self.config.proxy_config; + let mut providers = proxy_config.providers.clone(); + + let ic_client_bind_addr = "http://localhost:".to_owned() + port.to_string().as_str(); + let ic_client_bind_addr = ic_client_bind_addr.as_str(); + let client_api_uri = + url::Url::parse(ic_client_bind_addr).expect("Failed to parse replica ingress url."); + // Add the localhost as an option. + providers.push(client_api_uri); + eprintln!("replica address: {:?}", ic_client_bind_addr); + + let server = run_webserver( + proxy_config.logger.clone(), + proxy_config.build_output_root.clone(), + proxy_config.network_descriptor.clone(), + proxy_config.bind, + providers, + proxy_config.serve_dir.clone(), + )?; + + // webserver( + // proxy_config.logger.clone(), + // proxy_config.build_output_root.clone(), + // proxy_config.network_descriptor.clone(), + // proxy_config.bind, + // providers, + // &proxy_config.serve_dir, + // sender, + // )?.join() + // .map_err(|e| { + // DfxError::RuntimeError(Error::new( + // ErrorKind::Other, + // format!("Failed while running frontend proxy thead -- {:?}", e), + // )) + // })?; + // + // // Wait for the webserver to be started. + // let server = receiver.recv().expect("Failed to receive server..."); + + Ok(Some(server)) + } +} + +impl Actor for ReplicaWebserverCoordinator { + type Context = Context; + + fn started(&mut self, ctx: &mut Self::Context) { + info!(self.logger, "ReplicaWebserverCoordinator started"); + self.config + .replica_addr + .do_send(PortReadySubscribe(ctx.address().recipient())); + } +} + +impl Handler for ReplicaWebserverCoordinator { + type Result = (); + + fn handle(&mut self, msg: ReplicaReadySignal, ctx: &mut Self::Context) { + info!( + self.logger, + "ReplicaWebserverCoordinator: replica ready on {}", msg.port + ); + println!("replica ready {}", msg.port); + + if let Some(server) = &self.server { + ctx.wait(wrap_future(server.stop(true))); + self.server = None; + } + + let server = self.start_server(msg.port).unwrap(); + self.server = server; + } +} diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 997942ee9d..e23021df77 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -7,6 +7,9 @@ use crate::lib::proxy::{CoordinateProxy, ProxyConfig}; use crate::lib::proxy_process::spawn_and_update_proxy; use crate::lib::replica_config::ReplicaConfig; +use crate::actors; +use crate::actors::replica_webserver_coordinator::ReplicaWebserverCoordinator; +use actix::Actor; use clap::{App, Arg, ArgMatches, SubCommand}; use crossbeam::channel::{Receiver, Sender}; use crossbeam::unbounded; @@ -19,6 +22,7 @@ use std::io::{Error, ErrorKind}; use std::net::SocketAddr; use std::path::PathBuf; use std::process::Command; +//use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; use sysinfo::{Pid, Process, ProcessExt, Signal, System, SystemExt}; use tokio::runtime::Runtime; @@ -77,6 +81,10 @@ fn ping_and_wait(frontend_url: &str) -> DfxResult { }) } +fn never() -> bool { + false +} + // TODO(eftychis)/In progress: Rename to replica. /// Start the Internet Computer locally. Spawns a proxy to forward and /// manage browser requests. Responsible for running the network (one @@ -88,6 +96,147 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { let (frontend_url, address_and_port) = frontend_address(args, &config)?; + let replica_pathbuf = env.get_cache().get_binary_command_path("replica")?; + let ic_starter_pathbuf = env.get_cache().get_binary_command_path("ic-starter")?; + + let temp_dir = env.get_temp_dir(); + let state_root = env.get_state_dir(); + + let pid_file_path = temp_dir.join("pid"); + check_previous_process_running(&pid_file_path)?; + // As we know no start process is running in this project, we can + // clean up the state if it is necessary. + if args.is_present("clean") { + // Clean the contents of the provided directory including the + // directory itself. N.B. This does NOT follow symbolic links -- and I + // hope we do not need to. + if state_root.is_dir() { + fs::remove_dir_all(state_root.clone()).map_err(DfxError::CleanState)?; + } + let local_dir = temp_dir.join("local"); + if local_dir.is_dir() { + fs::remove_dir_all(local_dir).map_err(DfxError::CleanState)?; + } + } + + let client_configuration_dir = temp_dir.join("client-configuration"); + fs::create_dir_all(&client_configuration_dir)?; + let state_dir = temp_dir.join("state/replicated_state"); + fs::create_dir_all(&state_dir)?; + let client_port_path = client_configuration_dir.join("client-1.port"); + + // Touch the client port file. This ensures it is empty prior to + // handing it over to the replica. If we read the file and it has + // contents we shall assume it is due to our spawned client + // process. + std::fs::write(&client_port_path, "")?; + // We are doing this here to make sure we can write to the temp + // pid file. + std::fs::write(&pid_file_path, "")?; + + if args.is_present("background") { + send_background()?; + return ping_and_wait(&frontend_url); + } + + // Start the client. + let b = ProgressBar::new_spinner(); + b.set_draw_target(ProgressDrawTarget::stderr()); + + //b.set_message("Starting up the replica..."); + //b.enable_steady_tick(80); + + //b.set_message("Generating IC local replica configuration."); + let replica_config = ReplicaConfig::new(state_root).with_random_port(&client_port_path); + + let system = actix::System::new("dfx-start"); + + let replica_addr = actors::replica::Replica::new(actors::replica::Config { + ic_starter_path: ic_starter_pathbuf, + replica_config, + replica_path: replica_pathbuf, + logger: Some(env.get_logger().clone()), + }) + .start(); + + let network_descriptor = get_network_descriptor(env, args)?; + let bootstrap_dir = env.get_cache().get_binary_command_path("bootstrap")?; + // By default we reach to no external IC nodes. + let providers = Vec::new(); + let build_output_root = config + .get_temp_path() + .join(network_descriptor.name.clone()) + .join("canisters"); + + let proxy_config = ProxyConfig { + logger: env.get_logger().clone(), + client_api_port: address_and_port.port(), + bind: address_and_port, + serve_dir: bootstrap_dir, + providers, + build_output_root, + network_descriptor, + }; + + let _coordinator_actor = + ReplicaWebserverCoordinator::new(actors::replica_webserver_coordinator::Config { + replica_addr, + logger: Some(env.get_logger().clone()), + proxy_config, + }) + .start(); + + // Update the pid file. + if let Ok(pid) = sysinfo::get_current_pid() { + let _ = std::fs::write(&pid_file_path, pid.to_string()); + } + + println!("start signal watcher"); + + // let stopped = AtomicBool::new(false); + // ctrlc::set_handler(move || { + // println!("Ctrl-C received"); + // let already_stopped = stopped.compare_and_swap(false, true, Ordering::SeqCst); + // if already_stopped { + // println!(" - ignoring double hit"); + // } else { + // println!(" - (would) shutting down"); + // //actix::System::current().stop(); + // } + // }) + // .expect("Error setting ctrl-c handler"); + + // let ctrl_c = tokio_signal::ctrl_c().flatten_stream(); + // let handle_shutdown = ctrl_c.for_each(|()| { + // println!("Ctrl-C received, shutting down"); + // System::current().stop(); + // Ok(()) + // }).map_err(|_| ()); + // actix::spawn(handle_shutdown); + + //actors::signal_watcher::SignalWatchdog::new().start(); + println!("run system"); + system.run()?; + println!("system run returned"); + + if never() { + // so I don't have to delete a ton of dead code just yet + old_exec(env, &args)?; + } + Ok(()) +} + +// TODO(eftychis)/In progress: Rename to replica. +/// Start the Internet Computer locally. Spawns a proxy to forward and +/// manage browser requests. Responsible for running the network (one +/// replica at the moment) and the proxy. +pub fn old_exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { + let config = env + .get_config() + .ok_or(DfxError::CommandMustBeRunInAProject)?; + + let (frontend_url, address_and_port) = frontend_address(args, &config)?; + let client_pathbuf = env.get_cache().get_binary_command_path("replica")?; let ic_starter_pathbuf = env.get_cache().get_binary_command_path("ic-starter")?; diff --git a/src/dfx/src/lib/proxy.rs b/src/dfx/src/lib/proxy.rs index 4eab5d939a..d10b2d676b 100644 --- a/src/dfx/src/lib/proxy.rs +++ b/src/dfx/src/lib/proxy.rs @@ -90,16 +90,21 @@ impl Proxy { providers.push(client_api_uri); eprintln!("replica address: {:?}", ic_client_bind_addr); - run_webserver( + let server = run_webserver( self.config.logger.clone(), self.config.build_output_root.clone(), self.config.network_descriptor.clone(), self.config.bind, providers, self.config.serve_dir.clone(), - sender.clone(), + //sender.clone(), )?; + // // Warning: Note that HttpServer provides its own signal + // // handler. That means if we provide signal handling beyond basic + // // we need to either as normal "re-signal" or disable_signals(). + let _ = sender.clone().send(server); + let mut new_server = Proxy::new(self.config); let handle = ServerHandle { sender, receiver }; new_server.server_handle = ProxyServer::Up(handle); diff --git a/src/dfx/src/lib/webserver.rs b/src/dfx/src/lib/webserver.rs index bc038cf757..57b5cc242a 100644 --- a/src/dfx/src/lib/webserver.rs +++ b/src/dfx/src/lib/webserver.rs @@ -172,8 +172,8 @@ pub fn run_webserver( bind: SocketAddr, providers: Vec, serve_dir: PathBuf, - inform_parent: Sender, -) -> Result<(), std::io::Error> { + //inform_parent: Sender, +) -> Result { info!(logger, "binding to: {:?}", bind); const SHUTDOWN_WAIT_TIME: u64 = 60; @@ -189,7 +189,6 @@ pub fn run_webserver( .join(", ") ); - let _sys = System::new("dfx-frontend-http-server"); let forward_data = Arc::new(Mutex::new(ForwardActixData { providers, logger: logger.clone(), @@ -225,12 +224,12 @@ pub fn run_webserver( .system_exit() .run(); - // Warning: Note that HttpServer provides its own signal - // handler. That means if we provide signal handling beyond basic - // we need to either as normal "re-signal" or disable_signals(). - let _ = inform_parent.send(handler); + // // Warning: Note that HttpServer provides its own signal + // // handler. That means if we provide signal handling beyond basic + // // we need to either as normal "re-signal" or disable_signals(). + // let _ = inform_parent.send(handler); - Ok(()) + Ok(handler) } pub fn webserver( @@ -262,16 +261,22 @@ pub fn webserver( .spawn({ let serve_dir = serve_dir.to_path_buf(); move || { - run_webserver( + let _sys = System::new("dfx-frontend-http-server"); + let server = run_webserver( logger, build_output_root, network_descriptor, bind, clients_api_uri, serve_dir, - inform_parent, + //inform_parent, ) - .unwrap() + .unwrap(); + + // Warning: Note that HttpServer provides its own signal + // handler. That means if we provide signal handling beyond basic + // we need to either as normal "re-signal" or disable_signals(). + let _ = inform_parent.send(server); } }) .map_err(DfxError::from) From 93270d19222e74dab38ee236c4a6ae89920c3430 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 25 Sep 2020 15:17:19 -0700 Subject: [PATCH 02/50] remove dead code --- src/dfx/src/commands/start.rs | 347 +------------------------------ src/dfx/src/lib/error/mod.rs | 3 - src/dfx/src/lib/mod.rs | 1 - src/dfx/src/lib/proxy.rs | 113 +--------- src/dfx/src/lib/proxy_process.rs | 111 ---------- 5 files changed, 3 insertions(+), 572 deletions(-) delete mode 100644 src/dfx/src/lib/proxy_process.rs diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index e23021df77..4b7058aae4 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -3,28 +3,21 @@ use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; use crate::lib::message::UserMessage; use crate::lib::provider::get_network_descriptor; -use crate::lib::proxy::{CoordinateProxy, ProxyConfig}; -use crate::lib::proxy_process::spawn_and_update_proxy; +use crate::lib::proxy::ProxyConfig; use crate::lib::replica_config::ReplicaConfig; use crate::actors; use crate::actors::replica_webserver_coordinator::ReplicaWebserverCoordinator; use actix::Actor; use clap::{App, Arg, ArgMatches, SubCommand}; -use crossbeam::channel::{Receiver, Sender}; -use crossbeam::unbounded; use delay::{Delay, Waiter}; -use futures::executor::block_on; use ic_agent::{Agent, AgentConfig}; use indicatif::{ProgressBar, ProgressDrawTarget}; use std::fs; -use std::io::{Error, ErrorKind}; use std::net::SocketAddr; use std::path::PathBuf; use std::process::Command; -//use std::sync::atomic::{AtomicBool, Ordering}; -use std::time::Duration; -use sysinfo::{Pid, Process, ProcessExt, Signal, System, SystemExt}; +use sysinfo::{System, SystemExt}; use tokio::runtime::Runtime; /// Provide necessary arguments to start the Internet Computer @@ -81,10 +74,6 @@ fn ping_and_wait(frontend_url: &str) -> DfxResult { }) } -fn never() -> bool { - false -} - // TODO(eftychis)/In progress: Rename to replica. /// Start the Internet Computer locally. Spawns a proxy to forward and /// manage browser requests. Responsible for running the network (one @@ -191,237 +180,10 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { let _ = std::fs::write(&pid_file_path, pid.to_string()); } - println!("start signal watcher"); - - // let stopped = AtomicBool::new(false); - // ctrlc::set_handler(move || { - // println!("Ctrl-C received"); - // let already_stopped = stopped.compare_and_swap(false, true, Ordering::SeqCst); - // if already_stopped { - // println!(" - ignoring double hit"); - // } else { - // println!(" - (would) shutting down"); - // //actix::System::current().stop(); - // } - // }) - // .expect("Error setting ctrl-c handler"); - - // let ctrl_c = tokio_signal::ctrl_c().flatten_stream(); - // let handle_shutdown = ctrl_c.for_each(|()| { - // println!("Ctrl-C received, shutting down"); - // System::current().stop(); - // Ok(()) - // }).map_err(|_| ()); - // actix::spawn(handle_shutdown); - - //actors::signal_watcher::SignalWatchdog::new().start(); println!("run system"); system.run()?; println!("system run returned"); - if never() { - // so I don't have to delete a ton of dead code just yet - old_exec(env, &args)?; - } - Ok(()) -} - -// TODO(eftychis)/In progress: Rename to replica. -/// Start the Internet Computer locally. Spawns a proxy to forward and -/// manage browser requests. Responsible for running the network (one -/// replica at the moment) and the proxy. -pub fn old_exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { - let config = env - .get_config() - .ok_or(DfxError::CommandMustBeRunInAProject)?; - - let (frontend_url, address_and_port) = frontend_address(args, &config)?; - - let client_pathbuf = env.get_cache().get_binary_command_path("replica")?; - let ic_starter_pathbuf = env.get_cache().get_binary_command_path("ic-starter")?; - - let temp_dir = env.get_temp_dir(); - let state_root = env.get_state_dir(); - - let pid_file_path = temp_dir.join("pid"); - check_previous_process_running(&pid_file_path)?; - // As we know no start process is running in this project, we can - // clean up the state if it is necessary. - if args.is_present("clean") { - // Clean the contents of the provided directory including the - // directory itself. N.B. This does NOT follow symbolic links -- and I - // hope we do not need to. - if state_root.is_dir() { - fs::remove_dir_all(state_root.clone()).map_err(DfxError::CleanState)?; - } - let local_dir = temp_dir.join("local"); - if local_dir.is_dir() { - fs::remove_dir_all(local_dir).map_err(DfxError::CleanState)?; - } - } - - let client_configuration_dir = temp_dir.join("client-configuration"); - fs::create_dir_all(&client_configuration_dir)?; - let state_dir = temp_dir.join("state/replicated_state"); - fs::create_dir_all(&state_dir)?; - let client_port_path = client_configuration_dir.join("client-1.port"); - - // Touch the client port file. This ensures it is empty prior to - // handing it over to the replica. If we read the file and it has - // contents we shall assume it is due to our spawned client - // process. - std::fs::write(&client_port_path, "")?; - // We are doing this here to make sure we can write to the temp - // pid file. - std::fs::write(&pid_file_path, "")?; - - if args.is_present("background") { - send_background()?; - return ping_and_wait(&frontend_url); - } - - // Start the client. - let b = ProgressBar::new_spinner(); - b.set_draw_target(ProgressDrawTarget::stderr()); - - b.set_message("Starting up the replica..."); - b.enable_steady_tick(80); - - // Must be unbounded, as a killed child should not deadlock. - - let (request_stop, rcv_wait) = unbounded(); - let (broadcast_stop, is_killed_client) = unbounded(); - let (give_actix, actix_handler) = unbounded(); - - let request_stop_echo = request_stop.clone(); - let rcv_wait_fwatcher = rcv_wait.clone(); - b.set_message("Generating IC local replica configuration."); - let replica_config = ReplicaConfig::new(state_root).with_random_port(&client_port_path); - - // TODO(eftychis): we need a proper manager type when we start - // spawning multiple client processes and registry. - let client_watchdog = std::thread::Builder::new().name("replica".into()).spawn({ - let is_killed_client = is_killed_client.clone(); - let b = b.clone(); - - move || { - start_client( - &client_pathbuf, - &ic_starter_pathbuf, - &pid_file_path, - is_killed_client, - request_stop, - replica_config, - b, - ) - } - })?; - - let bootstrap_dir = env.get_cache().get_binary_command_path("bootstrap")?; - - // We have a long-lived replica process and a proxy. We use - // currently a messaging pattern to supervise. This is going to - // be tidied up over a more formal actor framework. - let is_killed = is_killed_client; - - // By default we reach to no external IC nodes. - let providers = Vec::new(); - - let network_descriptor = get_network_descriptor(env, args)?; - let build_output_root = config.get_temp_path().join(network_descriptor.name.clone()); - let build_output_root = build_output_root.join("canisters"); - - let proxy_config = ProxyConfig { - logger: env.get_logger().clone(), - client_api_port: address_and_port.port(), - bind: address_and_port, - serve_dir: bootstrap_dir, - providers, - build_output_root, - network_descriptor, - }; - - let supervisor_actor_handle = CoordinateProxy { - inform_parent: give_actix, - server_receiver: actix_handler.clone(), - rcv_wait_fwatcher, - request_stop_echo, - is_killed, - }; - - let frontend_watchdog = spawn_and_update_proxy( - proxy_config, - client_port_path, - supervisor_actor_handle, - b.clone(), - )?; - - b.set_message("Pinging the Internet Computer replica..."); - ping_and_wait(&frontend_url)?; - b.finish_with_message("Internet Computer replica started..."); - - // TODO/In Progress(eftychis): Here we should define a Supervisor - // actor to keep track and spawn these two processes - // independently. - - // We have two side processes involving multiple threads running at - // this point. We first wait for a signal that one of the processes - // terminated. N.B. We do not handle the case where the proxy - // terminates abruptly and we have to terminate the client as that - // complicates the situation right now, and we need a watcher that - // terminates all sibling processes if a process returns an error, - // which we lack. We consider this a fine trade-off for now. - - rcv_wait.recv().or_else(|e| { - Err(DfxError::RuntimeError(Error::new( - ErrorKind::Other, - format!("Failed while waiting for the manager -- {:?}", e), - ))) - })?; - - // Signal the client to stop. Right now we have little control - // over the client and nodemanager as it provides little - // handling. This is mostly done for completeness. In the future - // we should also force kill, if it ends up being necessary. - let b = ProgressBar::new_spinner(); - b.set_draw_target(ProgressDrawTarget::stderr()); - b.set_message("Terminating..."); - b.enable_steady_tick(80); - broadcast_stop.send(()).expect("Failed to signal children"); - // We can now start terminating our proxy server, we block to - // ensure termination is done properly. At this point the client - // is down though. - - // Signal the actix server to stop. This will - // block. - - b.set_message("Terminating proxy..."); - block_on( - actix_handler - .recv() - .expect("Failed to receive server") - .stop(true), - ); - - b.set_message("Gathering proxy thread..."); - // Join and handle errors for the frontend watchdog thread. - frontend_watchdog.join().map_err(|e| { - DfxError::RuntimeError(Error::new( - ErrorKind::Other, - format!("Failed while running frontend proxy thead -- {:?}", e), - )) - })?; - - b.set_message("Gathering replica thread..."); - // Join and handle errors for the client watchdog thread. Here we - // check the result of client_watchdog and start_client. - client_watchdog.join().map_err(|e| { - DfxError::RuntimeError(Error::new( - ErrorKind::Other, - format!("Failed while running replica thread -- {:?}", e), - )) - })??; - b.finish_with_message("Terminated successfully... Have a great day!!!"); Ok(()) } @@ -472,108 +234,3 @@ fn check_previous_process_running(dfx_pid_path: &PathBuf) -> DfxResult<()> { } Ok(()) } - -/// Starts the client. It is supposed to be used in a thread, thus -/// this function will panic when an error occurs that implies -/// termination of the replica and need the attention of the parent -/// thread. -/// -/// # Panics -/// We panic here to transmit an error to the parent thread. -fn start_client( - client_pathbuf: &PathBuf, - ic_starter_pathbuf: &PathBuf, - pid_file_path: &PathBuf, - is_killed_client: Receiver<()>, - request_stop: Sender<()>, - config: ReplicaConfig, - b: ProgressBar, -) -> DfxResult<()> { - b.set_message("Generating IC local replica configuration."); - - let ic_starter = ic_starter_pathbuf.as_path().as_os_str(); - let mut cmd = std::process::Command::new(ic_starter); - // if None is returned, then an empty String will be provided to replica-path - // TODO: figure out the right solution - cmd.args(&[ - "--replica-path", - client_pathbuf.to_str().unwrap_or_default(), - "--http-port-file", - config - .http_handler - .write_port_to - .unwrap_or_default() - .to_str() - .unwrap_or_default(), - "--state-dir", - config.state_manager.state_root.to_str().unwrap_or_default(), - "--require-valid-signatures", - ]); - cmd.stdout(std::process::Stdio::inherit()); - cmd.stderr(std::process::Stdio::inherit()); - - // If the replica itself fails, we are probably into deeper trouble than - // we can solve at this point and the user is better rerunning the server. - let mut child = cmd.spawn().unwrap_or_else(|e| { - request_stop - .try_send(()) - .expect("Replica thread couldn't signal parent to stop"); - // We still want to send an error message. - panic!("Couldn't spawn ic-starter with command {:?}: {}", cmd, e); - }); - - // Update the pid file. - if let Ok(pid) = sysinfo::get_current_pid() { - let _ = std::fs::write(&pid_file_path, pid.to_string()); - } - - // N.B. The logic below fixes errors from replica causing - // restarts. We do not want to respawn the replica on a failure. - // This should be substituted with a supervisor. - - // Did we receive a kill signal? - while is_killed_client.is_empty() { - // We have to wait for the child to exit here. We *should* - // always wait(). Read related documentation. - - // We check every 1s on the replica. This logic should be - // transferred / substituted by a supervisor object. - std::thread::sleep(Duration::from_millis(1000)); - - match child.try_wait() { - Ok(Some(status)) => { - // An error occurred: exit the loop. - b.set_message(format!("local replica exited with: {}", status).as_str()); - break; - } - Ok(None) => { - // No change in exit status. - continue; - } - Err(e) => { - request_stop - .send(()) - .expect("Could not signal parent thread from replica runner"); - panic!("Failed to check the status of the replica: {}", e) - } - } - } - // Terminate the replica; wait() then signal stop. Ignore errors - // -- we might get InvalidInput: that is fine -- process might - // have terminated already. - Process::new(child.id() as Pid, None, 0).kill(Signal::Term); - match child.wait() { - Ok(status) => b.set_message(format!("Replica exited with {}", status).as_str()), - Err(e) => b.set_message( - format!("Failed to properly wait for the replica to terminate {}", e).as_str(), - ), - } - - // We DO want to panic here, if we can not signal our - // parent. This is interpreted as an error via join by the - // parent thread. - request_stop - .send(()) - .expect("Could not signal parent thread from replica runner"); - Ok(()) -} diff --git a/src/dfx/src/lib/error/mod.rs b/src/dfx/src/lib/error/mod.rs index 562dfa9b2a..a4aabf43f5 100644 --- a/src/dfx/src/lib/error/mod.rs +++ b/src/dfx/src/lib/error/mod.rs @@ -93,9 +93,6 @@ pub enum DfxError { /// An error during parsing of a version string. VersionCouldNotBeParsed(semver::SemVerError), - /// String provided is not a port - CouldNotParsePort(std::num::ParseIntError), - /// A replica did not start successfully. ReplicaCouldNotBeStarted(), diff --git a/src/dfx/src/lib/mod.rs b/src/dfx/src/lib/mod.rs index 801bea92be..59d5bc3d0c 100644 --- a/src/dfx/src/lib/mod.rs +++ b/src/dfx/src/lib/mod.rs @@ -15,7 +15,6 @@ pub mod package_arguments; pub mod progress_bar; pub mod provider; pub mod proxy; -pub mod proxy_process; pub mod replica_config; pub mod waiter; pub mod webserver; diff --git a/src/dfx/src/lib/proxy.rs b/src/dfx/src/lib/proxy.rs index d10b2d676b..216036e014 100644 --- a/src/dfx/src/lib/proxy.rs +++ b/src/dfx/src/lib/proxy.rs @@ -1,9 +1,6 @@ use crate::lib::network::network_descriptor::NetworkDescriptor; -use crate::lib::webserver::run_webserver; use actix_server::Server; use crossbeam::channel::{Receiver, Sender}; -use std::io::Result; -use std::io::{Error, ErrorKind}; use std::net::SocketAddr; use std::path::PathBuf; @@ -28,118 +25,10 @@ pub struct ProxyConfig { } #[derive(Clone, Debug)] -enum ProxyServer { - Down, - Up(ServerHandle), -} +enum ProxyServer {} #[derive(Clone, Debug)] struct ServerHandle { sender: Sender, receiver: Receiver, } - -impl Proxy { - pub fn new(config: ProxyConfig) -> Self { - Self { - config, - server_handle: ProxyServer::Down, - } - } - - // Shutdown and start are private (for now). - async fn shutdown(self) -> Result { - match self.server_handle { - // In case the server is down we recall new() as in the - // future we might add more bookkeeping logic, which will - // end up in bugs. This makes this more readable to on - // what we want. The compiler can optimize this away. - ProxyServer::Down => Ok(Proxy::new(self.config)), - ProxyServer::Up(handler) => { - handler - .receiver - .try_recv() - .or_else(|e| { - Err(Error::new( - ErrorKind::Other, - format!("Failed to shutdown proxy -- {:?}", e), - )) - })? - .stop(true) - .await; - Ok(Self { - config: self.config, - server_handle: ProxyServer::Down, - }) - } - } - } - - /// Start a proxy with the provided configuration. Returns a proxy - /// handle. Can fail to return a new proxy. - /// # Panics - /// Currently, we panic if the underlying webserver does not start. - pub fn start(self, sender: Sender, receiver: Receiver) -> Result { - let mut providers = self.config.providers.clone(); - - let ic_client_bind_addr = "http://localhost:".to_owned() + self.port().to_string().as_str(); - let ic_client_bind_addr = ic_client_bind_addr.as_str(); - let client_api_uri = - url::Url::parse(ic_client_bind_addr).expect("Failed to parse replica ingress url."); - // Add the localhost as an option. - providers.push(client_api_uri); - eprintln!("replica address: {:?}", ic_client_bind_addr); - - let server = run_webserver( - self.config.logger.clone(), - self.config.build_output_root.clone(), - self.config.network_descriptor.clone(), - self.config.bind, - providers, - self.config.serve_dir.clone(), - //sender.clone(), - )?; - - // // Warning: Note that HttpServer provides its own signal - // // handler. That means if we provide signal handling beyond basic - // // we need to either as normal "re-signal" or disable_signals(). - let _ = sender.clone().send(server); - - let mut new_server = Proxy::new(self.config); - let handle = ServerHandle { sender, receiver }; - new_server.server_handle = ProxyServer::Up(handle); - Ok(new_server) - } - - /// Set the api port used by the replica. Returns a new proxy - /// object, but does not restart the proxy. - pub fn set_client_api_port(self, client_api_port: u16) -> Self { - let mut handle = self; - handle.config.client_api_port = client_api_port; - handle - } - - /// Restart a proxy with a new configuration. - pub async fn restart(self, sender: Sender, receiver: Receiver) -> Result { - let config = self.config.clone(); - let mut handle = self.shutdown().await?; - handle.config = config; - handle.start(sender, receiver) - } - - /// Return proxy client api port. - fn port(&self) -> u16 { - self.config.client_api_port - } -} - -/// Supervise a Proxy. -// This should be used to refactor and simplify handling of both proxy -// and replica. -pub struct CoordinateProxy { - pub inform_parent: Sender, - pub server_receiver: Receiver, - pub rcv_wait_fwatcher: Receiver<()>, - pub request_stop_echo: Sender<()>, - pub is_killed: Receiver<()>, -} diff --git a/src/dfx/src/lib/proxy_process.rs b/src/dfx/src/lib/proxy_process.rs deleted file mode 100644 index 60ed331200..0000000000 --- a/src/dfx/src/lib/proxy_process.rs +++ /dev/null @@ -1,111 +0,0 @@ -use crate::lib::error::DfxError; -use crate::lib::proxy::{CoordinateProxy, Proxy, ProxyConfig}; - -use crossbeam::unbounded; -use futures::executor::block_on; -use hotwatch::{Event, Hotwatch}; -use indicatif::ProgressBar; -use std::fs; -use std::path::PathBuf; -use std::time::Duration; - -pub fn spawn_and_update_proxy( - proxy_config: ProxyConfig, - client_port_path: PathBuf, - proxy_supervisor: CoordinateProxy, - b: ProgressBar, -) -> std::io::Result> { - std::thread::Builder::new() - .name("Proxy".into()) - .spawn(move || { - let proxy = Proxy::new(proxy_config); - // Start the proxy first. Below, we panic to propagate the error - // to the parent thread as an error via join(). - - // Check the port and then start the proxy. Below, we panic to propagate the error - // to the parent thread as an error via join(). - b.set_message("Checking replica!"); - - let (send_port, rcv_port) = unbounded(); - - let mut hotwatch = Hotwatch::new_with_custom_delay(std::time::Duration::from_secs(1)) - .expect("hotwatch failed to initialize!"); - let is_killed = proxy_supervisor.is_killed.clone(); - // We start a hotwatch watcher. It will run on the - // background, with "watch life" equal to the lifetime of - // the value. We attempt on each sensible event to read - // the port. - hotwatch - .watch(&client_port_path, { - let client_port_path = client_port_path.clone(); - move |event: Event| { - if !is_killed.is_empty() { - // We are in a weird state where the replica exited with an error, - // but we are still waiting for the pid file to change. As this change - // is never going to occur we need to exit our wait and stop tracking - // the file. We need to re-send the error to properly handle it later - // on. Worst case we will panic at this point. - // - // Disconnect the sender. This should unblock the port receiver. - let _ = send_port; - } - match event { - // We pretty much want to unblock for any events - // except a rescan. A move, create etc event should - // lead to a failure. - Event::Rescan => {} - _ => { - let port = fs::read_to_string(&client_port_path) - .map_err(DfxError::RuntimeError) - .expect("failed to read port file") - .parse::(); - if let Ok(port) = port { - send_port.send(port).expect("Failed to send port"); - } - } - } - } - }) - .expect("failed to watch replica port file!"); - - let port_after_enter = - fs::read_to_string(&client_port_path).unwrap_or_else(|_| "".to_owned()); - - let port = if port_after_enter != "" { - port_after_enter - .parse::() - .map_err(DfxError::CouldNotParsePort) - } else { - // Fail if sender is disconnected. - Ok(rcv_port.recv().expect("Failed to receive port")) - } - .unwrap_or_else(|e| { - proxy_supervisor - .request_stop_echo - .try_send(()) - .expect("Replica thread couldn't signal parent to stop"); - - panic!("Failed to watch port configuration file {:?}", e); - }); - // Stop watching. - let _ = hotwatch; - let proxy = proxy.set_client_api_port(port.clone()); - b.set_message(format!("Replica bound at {}", port).as_str()); - block_on(proxy.restart( - proxy_supervisor.inform_parent.clone(), - proxy_supervisor.server_receiver.clone(), - )) - .unwrap_or_else(|e| { - proxy_supervisor - .request_stop_echo - .try_send(()) - .expect("Replica thread couldn't signal parent to stop"); - panic!("Failed to restart the proxy {:?}", e); - }); - - while proxy_supervisor.is_killed.is_empty() { - std::thread::sleep(Duration::from_millis(1000)); - //wait! - } - }) -} From 1ee99b264c11b51d3bf4b578ce90db4250d41ca4 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 25 Sep 2020 16:33:21 -0700 Subject: [PATCH 03/50] extract methods; remove dead code; reorder, pass &Path not PathBuf --- src/dfx/src/commands/replica.rs | 2 +- src/dfx/src/commands/start.rs | 122 ++++++++++++++++++------------ src/dfx/src/lib/error/mod.rs | 2 +- src/dfx/src/lib/proxy.rs | 18 ----- src/dfx/src/lib/replica_config.rs | 2 +- src/dfx/src/lib/webserver.rs | 7 -- 6 files changed, 77 insertions(+), 76 deletions(-) diff --git a/src/dfx/src/commands/replica.rs b/src/dfx/src/commands/replica.rs index 04b34b85cc..d1be1e1ad5 100644 --- a/src/dfx/src/commands/replica.rs +++ b/src/dfx/src/commands/replica.rs @@ -58,7 +58,7 @@ fn get_config(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult DfxResult { }) } -// TODO(eftychis)/In progress: Rename to replica. /// Start the Internet Computer locally. Spawns a proxy to forward and /// manage browser requests. Responsible for running the network (one /// replica at the moment) and the proxy. @@ -85,49 +86,27 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { let (frontend_url, address_and_port) = frontend_address(args, &config)?; - let replica_pathbuf = env.get_cache().get_binary_command_path("replica")?; - let ic_starter_pathbuf = env.get_cache().get_binary_command_path("ic-starter")?; - let temp_dir = env.get_temp_dir(); let state_root = env.get_state_dir(); let pid_file_path = temp_dir.join("pid"); check_previous_process_running(&pid_file_path)?; + + if args.is_present("background") { + send_background()?; + return ping_and_wait(&frontend_url); + } + // As we know no start process is running in this project, we can // clean up the state if it is necessary. if args.is_present("clean") { - // Clean the contents of the provided directory including the - // directory itself. N.B. This does NOT follow symbolic links -- and I - // hope we do not need to. - if state_root.is_dir() { - fs::remove_dir_all(state_root.clone()).map_err(DfxError::CleanState)?; - } - let local_dir = temp_dir.join("local"); - if local_dir.is_dir() { - fs::remove_dir_all(local_dir).map_err(DfxError::CleanState)?; - } + clean_state(temp_dir, &state_root)?; } - let client_configuration_dir = temp_dir.join("client-configuration"); - fs::create_dir_all(&client_configuration_dir)?; - let state_dir = temp_dir.join("state/replicated_state"); - fs::create_dir_all(&state_dir)?; - let client_port_path = client_configuration_dir.join("client-1.port"); - - // Touch the client port file. This ensures it is empty prior to - // handing it over to the replica. If we read the file and it has - // contents we shall assume it is due to our spawned client - // process. - std::fs::write(&client_port_path, "")?; // We are doing this here to make sure we can write to the temp // pid file. std::fs::write(&pid_file_path, "")?; - if args.is_present("background") { - send_background()?; - return ping_and_wait(&frontend_url); - } - // Start the client. let b = ProgressBar::new_spinner(); b.set_draw_target(ProgressDrawTarget::stderr()); @@ -136,18 +115,75 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { //b.enable_steady_tick(80); //b.set_message("Generating IC local replica configuration."); - let replica_config = ReplicaConfig::new(state_root).with_random_port(&client_port_path); let system = actix::System::new("dfx-start"); - let replica_addr = actors::replica::Replica::new(actors::replica::Config { + let replica_addr = start_replica(env, &state_root)?; + + let _webserver_coordinator = + start_webserver_coordinator(env, args, config, address_and_port, replica_addr)?; + + // Update the pid file. + if let Ok(pid) = sysinfo::get_current_pid() { + let _ = std::fs::write(&pid_file_path, pid.to_string()); + } + + println!("run system"); + system.run()?; + println!("system run returned"); + + Ok(()) +} + +fn clean_state(temp_dir: &Path, state_root: &Path) -> DfxResult { + // Clean the contents of the provided directory including the + // directory itself. N.B. This does NOT follow symbolic links -- and I + // hope we do not need to. + if state_root.is_dir() { + fs::remove_dir_all(state_root) + .map_err(|e| DfxError::CleanState(e, PathBuf::from(state_root)))?; + } + let local_dir = temp_dir.join("local"); + if local_dir.is_dir() { + fs::remove_dir_all(&local_dir).map_err(|e| DfxError::CleanState(e, local_dir))?; + } + Ok(()) +} + +fn start_replica(env: &dyn Environment, state_root: &Path) -> DfxResult> { + let replica_pathbuf = env.get_cache().get_binary_command_path("replica")?; + let ic_starter_pathbuf = env.get_cache().get_binary_command_path("ic-starter")?; + + let temp_dir = env.get_temp_dir(); + let client_configuration_dir = temp_dir.join("client-configuration"); + fs::create_dir_all(&client_configuration_dir)?; + let state_dir = temp_dir.join("state/replicated_state"); + fs::create_dir_all(&state_dir)?; + let client_port_path = client_configuration_dir.join("client-1.port"); + + // Touch the client port file. This ensures it is empty prior to + // handing it over to the replica. If we read the file and it has + // contents we shall assume it is due to our spawned client + // process. + std::fs::write(&client_port_path, "")?; + + let replica_config = ReplicaConfig::new(state_root).with_random_port(&client_port_path); + Ok(actors::replica::Replica::new(actors::replica::Config { ic_starter_path: ic_starter_pathbuf, replica_config, replica_path: replica_pathbuf, logger: Some(env.get_logger().clone()), }) - .start(); + .start()) +} +fn start_webserver_coordinator( + env: &dyn Environment, + args: &ArgMatches<'_>, + config: Arc, + address_and_port: SocketAddr, + replica_addr: Addr, +) -> DfxResult> { let network_descriptor = get_network_descriptor(env, args)?; let bootstrap_dir = env.get_cache().get_binary_command_path("bootstrap")?; // By default we reach to no external IC nodes. @@ -167,24 +203,14 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { network_descriptor, }; - let _coordinator_actor = + Ok( ReplicaWebserverCoordinator::new(actors::replica_webserver_coordinator::Config { replica_addr, logger: Some(env.get_logger().clone()), proxy_config, }) - .start(); - - // Update the pid file. - if let Ok(pid) = sysinfo::get_current_pid() { - let _ = std::fs::write(&pid_file_path, pid.to_string()); - } - - println!("run system"); - system.run()?; - println!("system run returned"); - - Ok(()) + .start(), + ) } fn send_background() -> DfxResult<()> { diff --git a/src/dfx/src/lib/error/mod.rs b/src/dfx/src/lib/error/mod.rs index a4aabf43f5..d391a76421 100644 --- a/src/dfx/src/lib/error/mod.rs +++ b/src/dfx/src/lib/error/mod.rs @@ -79,7 +79,7 @@ pub enum DfxError { RuntimeError(std::io::Error), /// Failed to clean up state. - CleanState(std::io::Error), + CleanState(std::io::Error, PathBuf), /// The ide server shouldn't be started from a terminal. LanguageServerFromATerminal, diff --git a/src/dfx/src/lib/proxy.rs b/src/dfx/src/lib/proxy.rs index 216036e014..3e00b0d995 100644 --- a/src/dfx/src/lib/proxy.rs +++ b/src/dfx/src/lib/proxy.rs @@ -1,16 +1,7 @@ use crate::lib::network::network_descriptor::NetworkDescriptor; -use actix_server::Server; -use crossbeam::channel::{Receiver, Sender}; use std::net::SocketAddr; use std::path::PathBuf; -/// A proxy that forwards requests from the browser to the network. -#[derive(Clone, Debug)] -pub struct Proxy { - config: ProxyConfig, - server_handle: ProxyServer, -} - /// Provide basic information to the proxy about the API port, the /// address and the serve directory. #[derive(Clone, Debug)] @@ -23,12 +14,3 @@ pub struct ProxyConfig { pub build_output_root: PathBuf, pub network_descriptor: NetworkDescriptor, } - -#[derive(Clone, Debug)] -enum ProxyServer {} - -#[derive(Clone, Debug)] -struct ServerHandle { - sender: Sender, - receiver: Receiver, -} diff --git a/src/dfx/src/lib/replica_config.rs b/src/dfx/src/lib/replica_config.rs index 11798bf6bb..9420082675 100644 --- a/src/dfx/src/lib/replica_config.rs +++ b/src/dfx/src/lib/replica_config.rs @@ -58,7 +58,7 @@ pub struct ReplicaConfig { } impl ReplicaConfig { - pub fn new(state_root: PathBuf) -> Self { + pub fn new(state_root: &Path) -> Self { ReplicaConfig { http_handler: HttpHandlerConfig { write_port_to: None, diff --git a/src/dfx/src/lib/webserver.rs b/src/dfx/src/lib/webserver.rs index 57b5cc242a..42793ee9ab 100644 --- a/src/dfx/src/lib/webserver.rs +++ b/src/dfx/src/lib/webserver.rs @@ -172,7 +172,6 @@ pub fn run_webserver( bind: SocketAddr, providers: Vec, serve_dir: PathBuf, - //inform_parent: Sender, ) -> Result { info!(logger, "binding to: {:?}", bind); @@ -224,11 +223,6 @@ pub fn run_webserver( .system_exit() .run(); - // // Warning: Note that HttpServer provides its own signal - // // handler. That means if we provide signal handling beyond basic - // // we need to either as normal "re-signal" or disable_signals(). - // let _ = inform_parent.send(handler); - Ok(handler) } @@ -269,7 +263,6 @@ pub fn webserver( bind, clients_api_uri, serve_dir, - //inform_parent, ) .unwrap(); From b30f3869b068f54bc6ad1e27d58d36cd0b544f61 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 25 Sep 2020 17:07:31 -0700 Subject: [PATCH 04/50] remove unused filed ProxyConfig.client_api_port --- src/dfx/src/commands/start.rs | 1 - src/dfx/src/lib/proxy.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 72317c80de..9a9f331ab3 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -195,7 +195,6 @@ fn start_webserver_coordinator( let proxy_config = ProxyConfig { logger: env.get_logger().clone(), - client_api_port: address_and_port.port(), bind: address_and_port, serve_dir: bootstrap_dir, providers, diff --git a/src/dfx/src/lib/proxy.rs b/src/dfx/src/lib/proxy.rs index 3e00b0d995..53a2cbb24e 100644 --- a/src/dfx/src/lib/proxy.rs +++ b/src/dfx/src/lib/proxy.rs @@ -6,7 +6,6 @@ use std::path::PathBuf; /// address and the serve directory. #[derive(Clone, Debug)] pub struct ProxyConfig { - pub client_api_port: u16, pub bind: SocketAddr, pub serve_dir: PathBuf, pub providers: Vec, From 8c365f0200a45bb763f58184e64d6ac933601f8a Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 25 Sep 2020 17:25:50 -0700 Subject: [PATCH 05/50] Move ProxyConfig fields into replica webserver coordinator config (don't need two levels) --- .../actors/replica_webserver_coordinator.rs | 44 +++++++------------ src/dfx/src/commands/start.rs | 16 ++----- src/dfx/src/lib/mod.rs | 1 - src/dfx/src/lib/proxy.rs | 15 ------- 4 files changed, 19 insertions(+), 57 deletions(-) delete mode 100644 src/dfx/src/lib/proxy.rs diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 1d839fcd18..369d75fa04 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -2,17 +2,23 @@ use crate::actors::replica::signals::outbound::ReplicaReadySignal; use crate::actors::replica::signals::PortReadySubscribe; use crate::actors::replica::Replica; use crate::lib::error::DfxResult; -use crate::lib::proxy::ProxyConfig; +use crate::lib::network::network_descriptor::NetworkDescriptor; use crate::lib::webserver::run_webserver; use actix::fut::wrap_future; use actix::{Actor, Addr, AsyncContext, Context, Handler}; use actix_server::Server; use slog::{info, Logger}; +use std::net::SocketAddr; +use std::path::PathBuf; pub struct Config { - pub replica_addr: Addr, pub logger: Option, - pub proxy_config: ProxyConfig, + pub replica_addr: Addr, + pub bind: SocketAddr, + pub serve_dir: PathBuf, + pub providers: Vec, + pub build_output_root: PathBuf, + pub network_descriptor: NetworkDescriptor, } pub struct ReplicaWebserverCoordinator { @@ -33,8 +39,7 @@ impl ReplicaWebserverCoordinator { } fn start_server(&self, port: u16) -> DfxResult> { - let proxy_config = &self.config.proxy_config; - let mut providers = proxy_config.providers.clone(); + let mut providers = self.config.providers.clone(); let ic_client_bind_addr = "http://localhost:".to_owned() + port.to_string().as_str(); let ic_client_bind_addr = ic_client_bind_addr.as_str(); @@ -45,33 +50,14 @@ impl ReplicaWebserverCoordinator { eprintln!("replica address: {:?}", ic_client_bind_addr); let server = run_webserver( - proxy_config.logger.clone(), - proxy_config.build_output_root.clone(), - proxy_config.network_descriptor.clone(), - proxy_config.bind, + self.logger.clone(), + self.config.build_output_root.clone(), + self.config.network_descriptor.clone(), + self.config.bind, providers, - proxy_config.serve_dir.clone(), + self.config.serve_dir.clone(), )?; - // webserver( - // proxy_config.logger.clone(), - // proxy_config.build_output_root.clone(), - // proxy_config.network_descriptor.clone(), - // proxy_config.bind, - // providers, - // &proxy_config.serve_dir, - // sender, - // )?.join() - // .map_err(|e| { - // DfxError::RuntimeError(Error::new( - // ErrorKind::Other, - // format!("Failed while running frontend proxy thead -- {:?}", e), - // )) - // })?; - // - // // Wait for the webserver to be started. - // let server = receiver.recv().expect("Failed to receive server..."); - Ok(Some(server)) } } diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 9a9f331ab3..b52503430f 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -3,7 +3,6 @@ use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; use crate::lib::message::UserMessage; use crate::lib::provider::get_network_descriptor; -use crate::lib::proxy::ProxyConfig; use crate::lib::replica_config::ReplicaConfig; use crate::actors; @@ -193,23 +192,16 @@ fn start_webserver_coordinator( .join(network_descriptor.name.clone()) .join("canisters"); - let proxy_config = ProxyConfig { - logger: env.get_logger().clone(), + let coord_config = actors::replica_webserver_coordinator::Config { + logger: Some(env.get_logger().clone()), + replica_addr, bind: address_and_port, serve_dir: bootstrap_dir, providers, build_output_root, network_descriptor, }; - - Ok( - ReplicaWebserverCoordinator::new(actors::replica_webserver_coordinator::Config { - replica_addr, - logger: Some(env.get_logger().clone()), - proxy_config, - }) - .start(), - ) + Ok(ReplicaWebserverCoordinator::new(coord_config).start()) } fn send_background() -> DfxResult<()> { diff --git a/src/dfx/src/lib/mod.rs b/src/dfx/src/lib/mod.rs index 59d5bc3d0c..6c663a9ea6 100644 --- a/src/dfx/src/lib/mod.rs +++ b/src/dfx/src/lib/mod.rs @@ -14,7 +14,6 @@ pub mod operations; pub mod package_arguments; pub mod progress_bar; pub mod provider; -pub mod proxy; pub mod replica_config; pub mod waiter; pub mod webserver; diff --git a/src/dfx/src/lib/proxy.rs b/src/dfx/src/lib/proxy.rs deleted file mode 100644 index 53a2cbb24e..0000000000 --- a/src/dfx/src/lib/proxy.rs +++ /dev/null @@ -1,15 +0,0 @@ -use crate::lib::network::network_descriptor::NetworkDescriptor; -use std::net::SocketAddr; -use std::path::PathBuf; - -/// Provide basic information to the proxy about the API port, the -/// address and the serve directory. -#[derive(Clone, Debug)] -pub struct ProxyConfig { - pub bind: SocketAddr, - pub serve_dir: PathBuf, - pub providers: Vec, - pub logger: slog::Logger, - pub build_output_root: PathBuf, - pub network_descriptor: NetworkDescriptor, -} From 0ac8f04f8489ed4d2bd2222aec688ac290850dcb Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 25 Sep 2020 17:28:44 -0700 Subject: [PATCH 06/50] pass PathBuf as &Path --- src/dfx/src/commands/start.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index b52503430f..62be594ece 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -236,7 +236,7 @@ fn frontend_address(args: &ArgMatches<'_>, config: &Config) -> DfxResult<(String Ok((frontend_url, address_and_port)) } -fn check_previous_process_running(dfx_pid_path: &PathBuf) -> DfxResult<()> { +fn check_previous_process_running(dfx_pid_path: &Path) -> DfxResult<()> { if dfx_pid_path.exists() { // Read and verify it's not running. If it is just return. if let Ok(s) = std::fs::read_to_string(&dfx_pid_path) { From 35323ff7a786ea4430dc546a9e17a00ec1215de4 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Mon, 28 Sep 2020 17:19:04 -0700 Subject: [PATCH 07/50] omg wip --- e2e/bats/signals.bash | 42 ++++++++++++++++++ e2e/bats/start.bash | 43 +++++++++++++++++++ e2e/bats/utils/_.bash | 4 +- e2e/bats/utils/assertions.bash | 22 ++++++++++ src/dfx/src/actors/replica.rs | 28 ++++++++++-- .../actors/replica_webserver_coordinator.rs | 13 +++++- src/dfx/src/commands/start.rs | 12 ------ src/dfx/src/lib/webserver.rs | 2 +- 8 files changed, 145 insertions(+), 21 deletions(-) create mode 100644 e2e/bats/signals.bash create mode 100644 e2e/bats/start.bash diff --git a/e2e/bats/signals.bash b/e2e/bats/signals.bash new file mode 100644 index 0000000000..4d83cb13d2 --- /dev/null +++ b/e2e/bats/signals.bash @@ -0,0 +1,42 @@ +#!/usr/bin/env bats + +load utils/_ + +setup() { + # We want to work from a temporary directory, different for every test. + cd $(mktemp -d -t dfx-e2e-XXXXXXXX) + + dfx_new hello +} + +teardown() { + dfx_stop +} + +@test "dfx shuts down (gracefully) due to SIGINT" { + [ "$USE_IC_REF" ] && skip "skip for ic-ref" + + dfx_start + + DFX_PID=$(cat .dfx/pid) + + kill -SIGINT $DFX_PID + + assert_process_exits $DFX_PID 15s + + assert_no_dfx_start_or_replica_processes +} + +@test "dfx shuts down (gracefully) due to SIGTERM" { + [ "$USE_IC_REF" ] && skip "skip for ic-ref" + + dfx_start + + DFX_PID=$(cat .dfx/pid) + + kill -SIGTERM $DFX_PID + + assert_process_exits $DFX_PID 15s + + assert_no_dfx_start_or_replica_processes +} diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash new file mode 100644 index 0000000000..75757844d0 --- /dev/null +++ b/e2e/bats/start.bash @@ -0,0 +1,43 @@ +#!/usr/bin/env bats + +load utils/_ + +setup() { + # We want to work from a temporary directory, different for every test. + cd $(mktemp -d -t dfx-e2e-XXXXXXXX) + + dfx_new hello +} + +teardown() { + dfx_stop +} + +@test "dfx restarts the replica" { + [ "$USE_IC_REF" ] && skip "skip for ic-ref" + + dfx_start + + DFX_PID=$(cat .dfx/pid) + echo "dfx is process $DFX_PID" + + # find the replica that is the child of dfx. we do not have awk. + REPLICA_PID=$(ps -o "ppid, pid, comm" | grep ^$DFX_PID.*replica$ | cut -d ' ' -f 2) + echo "replica is process $REPLICA_PID" + + # not nice + kill -KILL $REPLICA_PID + + assert_process_exits $REPLICA_PID 15s + + timeout $timeout sh -c \ + "while ps -o "ppid, pid, comm" | grep ^$DFX_PID.*replica$; do echo waiting for replica to restart; sleep 1; done" \ + || (echo "replica did not restart" && ps aux && exit 1) + + ps aux | grep -e dfx -e replica + + echo "forcing failure" + exit 2 + +} + diff --git a/e2e/bats/utils/_.bash b/e2e/bats/utils/_.bash index 003e9c3cd1..f6bfb2ed14 100644 --- a/e2e/bats/utils/_.bash +++ b/e2e/bats/utils/_.bash @@ -82,8 +82,6 @@ dfx_stop() { local dfx_root=.dfx/ rm -rf $dfx_root - # Verify that processes are killed. - ! ( ps | grep "[/\s]dfx start" ) - ! ( ps | grep "[/\s]replica" ) + assert_no_dfx_start_or_replica_processes fi } diff --git a/e2e/bats/utils/assertions.bash b/e2e/bats/utils/assertions.bash index 12052a090f..8e74a27791 100644 --- a/e2e/bats/utils/assertions.bash +++ b/e2e/bats/utils/assertions.bash @@ -130,3 +130,25 @@ assert_neq() { | batslib_decorate "output does not match" \ | fail) } + + +# Asserts that a process exits within a timeframe +# Arguments: +# $1 - the PID +# $2 - the timeout +assert_process_exits() { + pid="$1" + timeout="$2" + + timeout $timeout sh -c \ + "while kill -0 $pid; do echo waiting for process $pid to exit; sleep 1; done" \ + || (echo "process $pid did not exit" && ps aux && exit 1) + +} + +# Asserts that `dfx start` and `replica` are no longer running +assert_no_dfx_start_or_replica_processes() { + # Verify that processes are killed. + ! ( ps | grep "[/\s]dfx start" ) + ! ( ps | grep "[/\s]replica" ) +} diff --git a/src/dfx/src/actors/replica.rs b/src/dfx/src/actors/replica.rs index a1ee8d227c..0c2e08e0f2 100644 --- a/src/dfx/src/actors/replica.rs +++ b/src/dfx/src/actors/replica.rs @@ -155,6 +155,9 @@ impl Actor for Replica { fn stopping(&mut self, _ctx: &mut Self::Context) -> Running { info!(self.logger, "Stopping the replica..."); + //info!(self.logger, "(would stop))"); + //std::thread::sleep(std::time::Duration::from_secs(86400)); + if let Some(sender) = self.stop_sender.take() { let _ = sender.send(()); } @@ -208,13 +211,31 @@ fn wait_for_child_or_receiver( receiver: &Receiver<()>, ) -> ChildOrReceiver { loop { + // let child_try_wait = child.try_wait(); + // let receiver_signalled = receiver.recv_timeout(std::time::Duration::from_millis(100)); + // + // match (receiver_signalled, child_try_wait) { + // (Ok(()), _) => { + // eprintln!("received from receiver"); + // std::thread::sleep(std::time::Duration::from_secs(86400)); + // return ChildOrReceiver::Receiver; + // }, + // (Err(_), Ok(Some(_))) => { + // eprintln!("child.try_wait returned Some"); + // std::thread::sleep(std::time::Duration::from_secs(86400)); + // return ChildOrReceiver::Child; + // }, + // _ => {} + // }; // Ping-pong between waiting 100 msec for a receiver signal, and check if // the child quit. if let Ok(()) = receiver.recv_timeout(std::time::Duration::from_millis(100)) { + eprintln!("received from receiver"); return ChildOrReceiver::Receiver; } if let Ok(Some(_)) = child.try_wait() { + eprintln!("child.try_wait returned Some"); return ChildOrReceiver::Child; } } @@ -261,6 +282,7 @@ fn replica_start_thread( ]); } + cmd.stdin(std::process::Stdio::null()); cmd.stdout(std::process::Stdio::inherit()); cmd.stderr(std::process::Stdio::inherit()); @@ -284,18 +306,18 @@ fn replica_start_thread( // We don't restart the replica if done = true. match wait_for_child_or_receiver(&mut child, &receiver) { ChildOrReceiver::Receiver => { - debug!(logger, "Got signal to stop. Killing replica process..."); + info!(logger, "Got signal to stop. Killing replica process..."); let _ = child.kill(); let _ = child.wait(); done = true; } ChildOrReceiver::Child => { - debug!(logger, "Replica process failed."); + info!(logger, "Replica process failed."); // Reset waiter if last start was over 2 seconds ago, and do not wait. if std::time::Instant::now().duration_since(last_start) >= Duration::from_secs(2) { - debug!( + info!( logger, "Last replica seemed to have been healthy, not waiting..." ); diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 369d75fa04..3220096d6f 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -10,6 +10,7 @@ use actix_server::Server; use slog::{info, Logger}; use std::net::SocketAddr; use std::path::PathBuf; +use actix::clock::{Duration, delay_for}; pub struct Config { pub logger: Option, @@ -84,11 +85,19 @@ impl Handler for ReplicaWebserverCoordinator { println!("replica ready {}", msg.port); if let Some(server) = &self.server { + println!("stopping webserver"); ctx.wait(wrap_future(server.stop(true))); self.server = None; + println!("delay before restarting webserver"); + ctx.wait(wrap_future(delay_for(Duration::from_secs(10)))); + ctx.address().do_send(ReplicaReadySignal { port: msg.port }); + } + else { + println!("starting webserver"); + + let server = self.start_server(msg.port).unwrap(); + self.server = server; } - let server = self.start_server(msg.port).unwrap(); - self.server = server; } } diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 62be594ece..48c2812ed7 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -12,7 +12,6 @@ use actix::{Actor, Addr}; use clap::{App, Arg, ArgMatches, SubCommand}; use delay::{Delay, Waiter}; use ic_agent::{Agent, AgentConfig}; -use indicatif::{ProgressBar, ProgressDrawTarget}; use std::fs; use std::net::SocketAddr; use std::path::{Path, PathBuf}; @@ -106,15 +105,6 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { // pid file. std::fs::write(&pid_file_path, "")?; - // Start the client. - let b = ProgressBar::new_spinner(); - b.set_draw_target(ProgressDrawTarget::stderr()); - - //b.set_message("Starting up the replica..."); - //b.enable_steady_tick(80); - - //b.set_message("Generating IC local replica configuration."); - let system = actix::System::new("dfx-start"); let replica_addr = start_replica(env, &state_root)?; @@ -127,9 +117,7 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { let _ = std::fs::write(&pid_file_path, pid.to_string()); } - println!("run system"); system.run()?; - println!("system run returned"); Ok(()) } diff --git a/src/dfx/src/lib/webserver.rs b/src/dfx/src/lib/webserver.rs index 42793ee9ab..9e74f4a5d8 100644 --- a/src/dfx/src/lib/webserver.rs +++ b/src/dfx/src/lib/webserver.rs @@ -220,7 +220,7 @@ pub fn run_webserver( .bind(bind)? // N.B. This is an arbitrary timeout for now. .shutdown_timeout(SHUTDOWN_WAIT_TIME) - .system_exit() + //.system_exit() .run(); Ok(handler) From 6650dfb9ebea609ba82c779d588e5fa9b575d8b4 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Mon, 5 Oct 2020 15:51:08 -0700 Subject: [PATCH 08/50] disable test in progress --- e2e/bats/start.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 75757844d0..f5a04dfe1b 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -15,6 +15,7 @@ teardown() { @test "dfx restarts the replica" { [ "$USE_IC_REF" ] && skip "skip for ic-ref" + skip "this test is not ready, nor is dfx start ready for it" dfx_start From 115f6a31570ecc650a5a3a5d32491fcaf8b4d38c Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Mon, 5 Oct 2020 16:16:29 -0700 Subject: [PATCH 09/50] format --- src/dfx/src/actors/replica_webserver_coordinator.rs | 6 ++---- src/dfx/src/commands/start.rs | 12 ++++++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 3220096d6f..a13a0b79f9 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -4,13 +4,13 @@ use crate::actors::replica::Replica; use crate::lib::error::DfxResult; use crate::lib::network::network_descriptor::NetworkDescriptor; use crate::lib::webserver::run_webserver; +use actix::clock::{delay_for, Duration}; use actix::fut::wrap_future; use actix::{Actor, Addr, AsyncContext, Context, Handler}; use actix_server::Server; use slog::{info, Logger}; use std::net::SocketAddr; use std::path::PathBuf; -use actix::clock::{Duration, delay_for}; pub struct Config { pub logger: Option, @@ -91,13 +91,11 @@ impl Handler for ReplicaWebserverCoordinator { println!("delay before restarting webserver"); ctx.wait(wrap_future(delay_for(Duration::from_secs(10)))); ctx.address().do_send(ReplicaReadySignal { port: msg.port }); - } - else { + } else { println!("starting webserver"); let server = self.start_server(msg.port).unwrap(); self.server = server; } - } } diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index d20923a3bb..f9dcc99687 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -9,6 +9,8 @@ use crate::util::get_reusable_socket_addr; use crate::actors; use crate::actors::replica::Replica; use crate::actors::replica_webserver_coordinator::ReplicaWebserverCoordinator; +use crate::actors::shutdown_controller; +use crate::actors::shutdown_controller::ShutdownController; use actix::{Actor, Addr}; use clap::{App, Arg, ArgMatches, SubCommand}; use delay::{Delay, Waiter}; @@ -21,8 +23,6 @@ use std::process::Command; use std::sync::Arc; use sysinfo::{System, SystemExt}; use tokio::runtime::Runtime; -use crate::actors::shutdown_controller::ShutdownController; -use crate::actors::shutdown_controller; /// Provide necessary arguments to start the Internet Computer /// locally. See `exec` for further information. @@ -160,7 +160,7 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { let shutdown_controller = ShutdownController::new(shutdown_controller::Config { logger: Some(env.get_logger().clone()), }) - .start(); + .start(); let replica_addr = start_replica(env, &state_root, shutdown_controller)?; @@ -192,7 +192,11 @@ fn clean_state(temp_dir: &Path, state_root: &Path) -> DfxResult { Ok(()) } -fn start_replica(env: &dyn Environment, state_root: &Path, shutdown_controller: Addr) -> DfxResult> { +fn start_replica( + env: &dyn Environment, + state_root: &Path, + shutdown_controller: Addr, +) -> DfxResult> { let replica_pathbuf = env.get_cache().get_binary_command_path("replica")?; let ic_starter_pathbuf = env.get_cache().get_binary_command_path("ic-starter")?; From c7b96d9b306307f04fa200aacd7638374e6411ab Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 12:56:55 -0700 Subject: [PATCH 10/50] checkpoint - e2e replica restart tests passes --- e2e/bats/start.bash | 25 +++++++++++++++---- e2e/bats/utils/_.bash | 9 +++++++ e2e/bats/utils/assertions.bash | 7 ++++++ .../actors/replica_webserver_coordinator.rs | 2 +- src/dfx/src/commands/stop.rs | 4 +++ 5 files changed, 41 insertions(+), 6 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index f5a04dfe1b..ff9c749a1e 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -15,30 +15,45 @@ teardown() { @test "dfx restarts the replica" { [ "$USE_IC_REF" ] && skip "skip for ic-ref" - skip "this test is not ready, nor is dfx start ready for it" + #skip "this test is not ready, nor is dfx start ready for it" dfx_start + install_asset greet + assert_command dfx deploy + + assert_command dfx canister call hello greet '("Alpha")' + assert_eq '("Hello, Alpha!")' + DFX_PID=$(cat .dfx/pid) echo "dfx is process $DFX_PID" + ps -o "ppid, pid, comm" # find the replica that is the child of dfx. we do not have awk. - REPLICA_PID=$(ps -o "ppid, pid, comm" | grep ^$DFX_PID.*replica$ | cut -d ' ' -f 2) + REPLICA_PID=$(ps -o "ppid, pid, comm" | grep ^\\s*$DFX_PID\\s.*replica$ | cut -d ' ' -f 2) echo "replica is process $REPLICA_PID" + ps -o "ppid, pid, comm" | grep -e replica -e dfx + # not nice kill -KILL $REPLICA_PID + echo "sent kill signal to replica $REPLICA_PID" assert_process_exits $REPLICA_PID 15s + echo "replica exited, waiting for replica to restart" + timeout $timeout sh -c \ - "while ps -o "ppid, pid, comm" | grep ^$DFX_PID.*replica$; do echo waiting for replica to restart; sleep 1; done" \ + 'until ps -o "ppid, pid, comm" | grep ^\\s*'$DFX_PID'\\s.*replica$; do echo waiting for replica to restart; sleep 1; done' \ || (echo "replica did not restart" && ps aux && exit 1) + echo "no longer waiting for replica to restart" + ps -o "ppid, pid, comm" + echo ps aux | grep -e dfx -e replica - echo "forcing failure" - exit 2 + assert_command dfx canister call hello greet '("Omega")' + assert_eq '("Hello, Omega!")' } diff --git a/e2e/bats/utils/_.bash b/e2e/bats/utils/_.bash index 2306f26910..c898384365 100644 --- a/e2e/bats/utils/_.bash +++ b/e2e/bats/utils/_.bash @@ -94,7 +94,16 @@ dfx_stop() { kill $(cat dfx-bootstrap.pid) rm -f dfx-bootstrap.pid else + echo "** process list before dfx stop" + ps | grep -e dfx -e replica + + echo ".dfx/pid is $(cat .dfx/pid)" + dfx stop + + echo "** process list after dfx stop" + ps | grep -e dfx -e replica + local dfx_root=.dfx/ rm -rf $dfx_root diff --git a/e2e/bats/utils/assertions.bash b/e2e/bats/utils/assertions.bash index 71878579de..3d0f21cb29 100644 --- a/e2e/bats/utils/assertions.bash +++ b/e2e/bats/utils/assertions.bash @@ -148,8 +148,15 @@ assert_process_exits() { # Asserts that `dfx start` and `replica` are no longer running assert_no_dfx_start_or_replica_processes() { + ps | grep -e dfx -e replica + + echo "check for (space)dfx start" ! ( ps | grep "[/\s]dfx start" ) + echo "check for (space)replica" ! ( ps | grep "[/\s]replica" ) + + echo "check for (anything)dfx start" + ! ( ps | grep -v grep | grep "dfx start" ) } assert_file_eventually_exists() { diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index a13a0b79f9..5736f51a51 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -89,7 +89,7 @@ impl Handler for ReplicaWebserverCoordinator { ctx.wait(wrap_future(server.stop(true))); self.server = None; println!("delay before restarting webserver"); - ctx.wait(wrap_future(delay_for(Duration::from_secs(10)))); + ctx.wait(wrap_future(delay_for(Duration::from_millis(100)))); ctx.address().do_send(ReplicaReadySignal { port: msg.port }); } else { println!("starting webserver"); diff --git a/src/dfx/src/commands/stop.rs b/src/dfx/src/commands/stop.rs index a9f3c3f48c..643c7e8f70 100644 --- a/src/dfx/src/commands/stop.rs +++ b/src/dfx/src/commands/stop.rs @@ -26,8 +26,12 @@ fn list_all_descendants(pid: Pid) -> Vec { /// Recursively kill a process and ALL its children. fn kill_all(pid: Pid) -> DfxResult { + eprintln!("kill pid {} and all descendants", pid); let processes = list_all_descendants(pid); + eprintln!(" - descendants: {:?}", processes); for pid in processes { + eprintln!(" - kill process {}", pid); + let process = Process::new(pid, None, 0); process.kill(Signal::Term); } From 388d7bb7e9bd5d564ff9732d804e6af18aec64a3 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 14:24:09 -0700 Subject: [PATCH 11/50] cleanup --- src/dfx/src/actors/replica.rs | 2 - .../actors/replica_webserver_coordinator.rs | 49 ++++++++++--------- src/dfx/src/lib/webserver.rs | 2 +- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/src/dfx/src/actors/replica.rs b/src/dfx/src/actors/replica.rs index 8630badbf0..c8d4a24147 100644 --- a/src/dfx/src/actors/replica.rs +++ b/src/dfx/src/actors/replica.rs @@ -136,7 +136,6 @@ impl Replica { addr, receiver, )?; - info!(self.logger, "started replica thread"); self.thread_join = Some(handle); self.stop_sender = Some(sender); @@ -157,7 +156,6 @@ impl Actor for Replica { fn started(&mut self, ctx: &mut Self::Context) { self.start_replica(ctx.address()) .expect("Could not start the replica"); - info!(self.logger, "Replica Actor started..."); self.config .shutdown_controller diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 5736f51a51..7598839d34 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -8,7 +8,7 @@ use actix::clock::{delay_for, Duration}; use actix::fut::wrap_future; use actix::{Actor, Addr, AsyncContext, Context, Handler}; use actix_server::Server; -use slog::{info, Logger}; +use slog::{debug, error, info, Logger}; use std::net::SocketAddr; use std::path::PathBuf; @@ -22,6 +22,10 @@ pub struct Config { pub network_descriptor: NetworkDescriptor, } +/// +/// The ReplicaWebserverCoordinator runs a webserver for the replica. +/// +/// If the replica restarts, it will start a new webserver for the new replica. pub struct ReplicaWebserverCoordinator { logger: Logger, config: Config, @@ -39,27 +43,27 @@ impl ReplicaWebserverCoordinator { } } - fn start_server(&self, port: u16) -> DfxResult> { + fn start_server(&self, port: u16) -> DfxResult { let mut providers = self.config.providers.clone(); let ic_client_bind_addr = "http://localhost:".to_owned() + port.to_string().as_str(); let ic_client_bind_addr = ic_client_bind_addr.as_str(); let client_api_uri = url::Url::parse(ic_client_bind_addr).expect("Failed to parse replica ingress url."); - // Add the localhost as an option. providers.push(client_api_uri); - eprintln!("replica address: {:?}", ic_client_bind_addr); + info!( + self.logger, + "Starting webserver on port {} for replica at {:?}", port, ic_client_bind_addr + ); - let server = run_webserver( + run_webserver( self.logger.clone(), self.config.build_output_root.clone(), self.config.network_descriptor.clone(), self.config.bind, providers, self.config.serve_dir.clone(), - )?; - - Ok(Some(server)) + ) } } @@ -67,7 +71,6 @@ impl Actor for ReplicaWebserverCoordinator { type Context = Context; fn started(&mut self, ctx: &mut Self::Context) { - info!(self.logger, "ReplicaWebserverCoordinator started"); self.config .replica_addr .do_send(PortReadySubscribe(ctx.address().recipient())); @@ -78,24 +81,26 @@ impl Handler for ReplicaWebserverCoordinator { type Result = (); fn handle(&mut self, msg: ReplicaReadySignal, ctx: &mut Self::Context) { - info!( - self.logger, - "ReplicaWebserverCoordinator: replica ready on {}", msg.port - ); - println!("replica ready {}", msg.port); + debug!(self.logger, "replica ready on {}", msg.port); if let Some(server) = &self.server { - println!("stopping webserver"); ctx.wait(wrap_future(server.stop(true))); self.server = None; - println!("delay before restarting webserver"); - ctx.wait(wrap_future(delay_for(Duration::from_millis(100)))); - ctx.address().do_send(ReplicaReadySignal { port: msg.port }); + ctx.address().do_send(msg); } else { - println!("starting webserver"); - - let server = self.start_server(msg.port).unwrap(); - self.server = server; + match self.start_server(msg.port) { + Ok(server) => { + self.server = Some(server); + } + Err(e) => { + error!( + self.logger, + "Unable to start webserver on port {}: {}", msg.port, e + ); + ctx.wait(wrap_future(delay_for(Duration::from_secs(2)))); + ctx.address().do_send(msg); + } + } } } } diff --git a/src/dfx/src/lib/webserver.rs b/src/dfx/src/lib/webserver.rs index 9e74f4a5d8..3227d49dd2 100644 --- a/src/dfx/src/lib/webserver.rs +++ b/src/dfx/src/lib/webserver.rs @@ -172,7 +172,7 @@ pub fn run_webserver( bind: SocketAddr, providers: Vec, serve_dir: PathBuf, -) -> Result { +) -> DfxResult { info!(logger, "binding to: {:?}", bind); const SHUTDOWN_WAIT_TIME: u64 = 60; From 5ec77fd00da92c53c3646da242298bb311c279b3 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 17:18:28 -0700 Subject: [PATCH 12/50] cleanup --- e2e/bats/start.bash | 20 +---------- e2e/bats/utils/_.bash | 9 ----- e2e/bats/utils/assertions.bash | 7 ---- src/dfx/src/actors/replica.rs | 61 +++++++++++----------------------- src/dfx/src/commands/stop.rs | 4 --- src/dfx/src/lib/webserver.rs | 1 - 6 files changed, 20 insertions(+), 82 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index ff9c749a1e..f9de28916c 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -10,50 +10,32 @@ setup() { } teardown() { - dfx_stop + dfx_stop } @test "dfx restarts the replica" { [ "$USE_IC_REF" ] && skip "skip for ic-ref" - #skip "this test is not ready, nor is dfx start ready for it" dfx_start install_asset greet assert_command dfx deploy - assert_command dfx canister call hello greet '("Alpha")' assert_eq '("Hello, Alpha!")' DFX_PID=$(cat .dfx/pid) - echo "dfx is process $DFX_PID" - ps -o "ppid, pid, comm" # find the replica that is the child of dfx. we do not have awk. REPLICA_PID=$(ps -o "ppid, pid, comm" | grep ^\\s*$DFX_PID\\s.*replica$ | cut -d ' ' -f 2) - echo "replica is process $REPLICA_PID" - - ps -o "ppid, pid, comm" | grep -e replica -e dfx - # not nice kill -KILL $REPLICA_PID - echo "sent kill signal to replica $REPLICA_PID" - assert_process_exits $REPLICA_PID 15s - echo "replica exited, waiting for replica to restart" - timeout $timeout sh -c \ 'until ps -o "ppid, pid, comm" | grep ^\\s*'$DFX_PID'\\s.*replica$; do echo waiting for replica to restart; sleep 1; done' \ || (echo "replica did not restart" && ps aux && exit 1) - echo "no longer waiting for replica to restart" - ps -o "ppid, pid, comm" - echo - ps aux | grep -e dfx -e replica - assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' - } diff --git a/e2e/bats/utils/_.bash b/e2e/bats/utils/_.bash index c898384365..2306f26910 100644 --- a/e2e/bats/utils/_.bash +++ b/e2e/bats/utils/_.bash @@ -94,16 +94,7 @@ dfx_stop() { kill $(cat dfx-bootstrap.pid) rm -f dfx-bootstrap.pid else - echo "** process list before dfx stop" - ps | grep -e dfx -e replica - - echo ".dfx/pid is $(cat .dfx/pid)" - dfx stop - - echo "** process list after dfx stop" - ps | grep -e dfx -e replica - local dfx_root=.dfx/ rm -rf $dfx_root diff --git a/e2e/bats/utils/assertions.bash b/e2e/bats/utils/assertions.bash index 3d0f21cb29..71878579de 100644 --- a/e2e/bats/utils/assertions.bash +++ b/e2e/bats/utils/assertions.bash @@ -148,15 +148,8 @@ assert_process_exits() { # Asserts that `dfx start` and `replica` are no longer running assert_no_dfx_start_or_replica_processes() { - ps | grep -e dfx -e replica - - echo "check for (space)dfx start" ! ( ps | grep "[/\s]dfx start" ) - echo "check for (space)replica" ! ( ps | grep "[/\s]replica" ) - - echo "check for (anything)dfx start" - ! ( ps | grep -v grep | grep "dfx start" ) } assert_file_eventually_exists() { diff --git a/src/dfx/src/actors/replica.rs b/src/dfx/src/actors/replica.rs index c8d4a24147..c79e0e2375 100644 --- a/src/dfx/src/actors/replica.rs +++ b/src/dfx/src/actors/replica.rs @@ -97,12 +97,10 @@ impl Replica { .timeout(Duration::from_secs(30)) .build(); - println!("wait_for_port_file({})", file_path.to_string_lossy()); waiter.start(); loop { if let Ok(content) = std::fs::read_to_string(file_path) { if let Ok(port) = content.parse::() { - println!(" - got port {}", port); return Ok(port); } } @@ -143,7 +141,6 @@ impl Replica { } fn send_ready_signal(&self, port: u16) { - info!(self.logger, "send_ready_signal(port={})", port); for sub in &self.ready_subscribers { let _ = sub.do_send(signals::outbound::ReplicaReadySignal { port }); } @@ -164,9 +161,6 @@ impl Actor for Replica { fn stopping(&mut self, _ctx: &mut Self::Context) -> Running { info!(self.logger, "Stopping the replica..."); - //info!(self.logger, "(would stop))"); - //std::thread::sleep(std::time::Duration::from_secs(86400)); - if let Some(sender) = self.stop_sender.take() { let _ = sender.send(()); } @@ -199,10 +193,6 @@ impl Handler for Replica { type Result = (); fn handle(&mut self, msg: ReplicaRestarted, _ctx: &mut Self::Context) -> Self::Result { - info!( - self.logger, - "Replica::Handler(ReplicaRestarted port={})", msg.port - ); self.port = Some(msg.port); self.send_ready_signal(msg.port); } @@ -236,33 +226,22 @@ fn wait_for_child_or_receiver( receiver: &Receiver<()>, ) -> ChildOrReceiver { loop { - // let child_try_wait = child.try_wait(); - // let receiver_signalled = receiver.recv_timeout(std::time::Duration::from_millis(100)); - // - // match (receiver_signalled, child_try_wait) { - // (Ok(()), _) => { - // eprintln!("received from receiver"); - // std::thread::sleep(std::time::Duration::from_secs(86400)); - // return ChildOrReceiver::Receiver; - // }, - // (Err(_), Ok(Some(_))) => { - // eprintln!("child.try_wait returned Some"); - // std::thread::sleep(std::time::Duration::from_secs(86400)); - // return ChildOrReceiver::Child; - // }, - // _ => {} - // }; - // Ping-pong between waiting 100 msec for a receiver signal, and check if - // the child quit. - if let Ok(()) = receiver.recv_timeout(std::time::Duration::from_millis(100)) { - eprintln!("received from receiver"); - return ChildOrReceiver::Receiver; - } - - if let Ok(Some(_)) = child.try_wait() { - eprintln!("child.try_wait returned Some"); - return ChildOrReceiver::Child; - } + // Check if either the child exited or a shutdown has been requested. + // These can happen in either order in response to Ctrl-C, so increase the chance + // to notice a shutdown request even if the replica exited quickly. + let child_try_wait = child.try_wait(); + let receiver_signalled = receiver.recv_timeout(std::time::Duration::from_millis(100)); + + match (receiver_signalled, child_try_wait) { + (Ok(()), _) => { + // Prefer to indicate the shutdown request + return ChildOrReceiver::Receiver; + } + (Err(_), Ok(Some(_))) => { + return ChildOrReceiver::Child; + } + _ => {} + }; } } @@ -322,26 +301,24 @@ fn replica_start_thread( let port = port.unwrap_or_else(|| { Replica::wait_for_port_file(write_port_to.as_ref().unwrap()).unwrap() }); - println!("sending ReplicaRestarted"); addr.do_send(signals::ReplicaRestarted { port }); - println!("wait_for_child_or_receiver..."); // This waits for the child to stop, or the receiver to receive a message. // We don't restart the replica if done = true. match wait_for_child_or_receiver(&mut child, &receiver) { ChildOrReceiver::Receiver => { - info!(logger, "Got signal to stop. Killing replica process..."); + debug!(logger, "Got signal to stop. Killing replica process..."); let _ = child.kill(); let _ = child.wait(); done = true; } ChildOrReceiver::Child => { - info!(logger, "Replica process failed."); + debug!(logger, "Replica process failed."); // Reset waiter if last start was over 2 seconds ago, and do not wait. if std::time::Instant::now().duration_since(last_start) >= Duration::from_secs(2) { - info!( + debug!( logger, "Last replica seemed to have been healthy, not waiting..." ); diff --git a/src/dfx/src/commands/stop.rs b/src/dfx/src/commands/stop.rs index 643c7e8f70..a9f3c3f48c 100644 --- a/src/dfx/src/commands/stop.rs +++ b/src/dfx/src/commands/stop.rs @@ -26,12 +26,8 @@ fn list_all_descendants(pid: Pid) -> Vec { /// Recursively kill a process and ALL its children. fn kill_all(pid: Pid) -> DfxResult { - eprintln!("kill pid {} and all descendants", pid); let processes = list_all_descendants(pid); - eprintln!(" - descendants: {:?}", processes); for pid in processes { - eprintln!(" - kill process {}", pid); - let process = Process::new(pid, None, 0); process.kill(Signal::Term); } diff --git a/src/dfx/src/lib/webserver.rs b/src/dfx/src/lib/webserver.rs index 3227d49dd2..2f827c5658 100644 --- a/src/dfx/src/lib/webserver.rs +++ b/src/dfx/src/lib/webserver.rs @@ -220,7 +220,6 @@ pub fn run_webserver( .bind(bind)? // N.B. This is an arbitrary timeout for now. .shutdown_timeout(SHUTDOWN_WAIT_TIME) - //.system_exit() .run(); Ok(handler) From 2b2ab33545da44ddd8f9b393a8d21face204e85a Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 17:57:07 -0700 Subject: [PATCH 13/50] oh boy. in a nix shell, ps has different output/options for darwin/linux --- e2e/bats/start.bash | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index f9de28916c..f43590fa5b 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -25,6 +25,12 @@ teardown() { DFX_PID=$(cat .dfx/pid) + # this differs between linux and darwin under nix? + echo "ps" + ps + echo "ps --help" + ps --help + # find the replica that is the child of dfx. we do not have awk. REPLICA_PID=$(ps -o "ppid, pid, comm" | grep ^\\s*$DFX_PID\\s.*replica$ | cut -d ' ' -f 2) From 87230b1b7971ec85c0461da8511d9ccace4363da Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 18:05:35 -0700 Subject: [PATCH 14/50] try to see what blessings linux ps has to offer ffs --- e2e/bats/start.bash | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index f43590fa5b..f4e71e777b 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -28,9 +28,22 @@ teardown() { # this differs between linux and darwin under nix? echo "ps" ps - echo "ps --help" - ps --help - + echo "man ps" + man ps + echo "ps simple" + ps simple + echo "ps list" + ps list + echo "ps output" + ps output + echo "ps threads" + ps threads + echo "ps misc" + ps misc + echo "ps all" + ps all + + ps -o "ppid, pid, comm" # find the replica that is the child of dfx. we do not have awk. REPLICA_PID=$(ps -o "ppid, pid, comm" | grep ^\\s*$DFX_PID\\s.*replica$ | cut -d ' ' -f 2) From 3c93d7689773ebf540fe10390ee57c2b68525818 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 18:07:13 -0700 Subject: [PATCH 15/50] no man; --- e2e/bats/start.bash | 2 -- 1 file changed, 2 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index f4e71e777b..0b9c409dcf 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -28,8 +28,6 @@ teardown() { # this differs between linux and darwin under nix? echo "ps" ps - echo "man ps" - man ps echo "ps simple" ps simple echo "ps list" From c09fa7c810ab50c7488838a8be736d6882d37b6d Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 18:08:53 -0700 Subject: [PATCH 16/50] error output so helpful --- e2e/bats/start.bash | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 0b9c409dcf..b77d07c6e5 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -29,17 +29,17 @@ teardown() { echo "ps" ps echo "ps simple" - ps simple + ps s echo "ps list" - ps list + ps l echo "ps output" - ps output + ps o echo "ps threads" - ps threads + ps t echo "ps misc" - ps misc + ps m echo "ps all" - ps all + ps a ps -o "ppid, pid, comm" # find the replica that is the child of dfx. we do not have awk. From e0758475409803914cdda3cb6a5ab7901a284ff3 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 18:17:23 -0700 Subject: [PATCH 17/50] use pattern from assertion; --- e2e/bats/start.bash | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index b77d07c6e5..98a7ea41a3 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -25,31 +25,19 @@ teardown() { DFX_PID=$(cat .dfx/pid) - # this differs between linux and darwin under nix? - echo "ps" - ps - echo "ps simple" - ps s - echo "ps list" - ps l - echo "ps output" - ps o - echo "ps threads" - ps t - echo "ps misc" - ps m - echo "ps all" - ps a - - ps -o "ppid, pid, comm" + ps | grep replica + ps | grep "[/\s]replica" + # find the replica that is the child of dfx. we do not have awk. - REPLICA_PID=$(ps -o "ppid, pid, comm" | grep ^\\s*$DFX_PID\\s.*replica$ | cut -d ' ' -f 2) + REPLICA_PID=$(ps | grep "[/\s]replica" | cut -d ' ' -f 1) + + echo "replica pid is $REPLICA_PID" kill -KILL $REPLICA_PID assert_process_exits $REPLICA_PID 15s timeout $timeout sh -c \ - 'until ps -o "ppid, pid, comm" | grep ^\\s*'$DFX_PID'\\s.*replica$; do echo waiting for replica to restart; sleep 1; done' \ + 'until ps | grep "[/\s]replica"; do echo waiting for replica to restart; sleep 1; done' \ || (echo "replica did not restart" && ps aux && exit 1) assert_command dfx canister call hello greet '("Omega")' From 088248d2bfc81470ca4989979a66aaca3680fe04 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Tue, 6 Oct 2020 18:21:34 -0700 Subject: [PATCH 18/50] ps greppppppp --- e2e/bats/start.bash | 1 - 1 file changed, 1 deletion(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 98a7ea41a3..08fd870972 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -26,7 +26,6 @@ teardown() { DFX_PID=$(cat .dfx/pid) ps | grep replica - ps | grep "[/\s]replica" # find the replica that is the child of dfx. we do not have awk. REPLICA_PID=$(ps | grep "[/\s]replica" | cut -d ' ' -f 1) From 24aed0b22acb63feed64acfcdf05fa6f4b43394a Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 10:14:29 -0700 Subject: [PATCH 19/50] simpler --- e2e/bats/start.bash | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 08fd870972..6dde884146 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -25,7 +25,13 @@ teardown() { DFX_PID=$(cat .dfx/pid) + echo "xx 1" ps | grep replica + echo "xx 2" + ps | grep "[/\s]replica" + echo "xx 3" + ps | grep "[/\s]replica" | cut -d ' ' -f 1 + echo "xx 4" # find the replica that is the child of dfx. we do not have awk. REPLICA_PID=$(ps | grep "[/\s]replica" | cut -d ' ' -f 1) @@ -39,7 +45,13 @@ teardown() { 'until ps | grep "[/\s]replica"; do echo waiting for replica to restart; sleep 1; done' \ || (echo "replica did not restart" && ps aux && exit 1) + echo + ps | grep "[/\s]replica" + echo + assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' + + #assert_match "what" } From 109453a8fac0c61eafa1d4db15a761edaa746527 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 10:52:23 -0700 Subject: [PATCH 20/50] egrep with :space: ? --- e2e/bats/start.bash | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 6dde884146..6b699e59a9 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -25,12 +25,14 @@ teardown() { DFX_PID=$(cat .dfx/pid) + echo "xx 0" + ps echo "xx 1" - ps | grep replica + ps | egrep replica echo "xx 2" - ps | grep "[/\s]replica" + ps | egrep "[/[:space:]]replica" echo "xx 3" - ps | grep "[/\s]replica" | cut -d ' ' -f 1 + ps | egrep "[/\s]replica" | cut -d ' ' -f 1 echo "xx 4" # find the replica that is the child of dfx. we do not have awk. From 538ba6a119e3a5332e139e941dc198ad7e24bb9f Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:03:52 -0700 Subject: [PATCH 21/50] try grep with :space: --- e2e/bats/start.bash | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 6b699e59a9..a441407e44 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -28,32 +28,31 @@ teardown() { echo "xx 0" ps echo "xx 1" - ps | egrep replica + ps | grep replica echo "xx 2" - ps | egrep "[/[:space:]]replica" + ps | grep [/[:space:]]replica echo "xx 3" - ps | egrep "[/\s]replica" | cut -d ' ' -f 1 + ps | grep [/[:space:]]replica | cut -d ' ' -f 1 echo "xx 4" # find the replica that is the child of dfx. we do not have awk. - REPLICA_PID=$(ps | grep "[/\s]replica" | cut -d ' ' -f 1) + REPLICA_PID=$(ps | grep [/[:space:]]replica | cut -d ' ' -f 1) echo "replica pid is $REPLICA_PID" kill -KILL $REPLICA_PID assert_process_exits $REPLICA_PID 15s + echo "replica exited" + timeout $timeout sh -c \ - 'until ps | grep "[/\s]replica"; do echo waiting for replica to restart; sleep 1; done' \ + 'until ps | grep [/[:space:]]replica; do echo waiting for replica to restart; sleep 1; done' \ || (echo "replica did not restart" && ps aux && exit 1) - echo - ps | grep "[/\s]replica" - echo + echo "replica restarted" + sleep 10 assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' - - #assert_match "what" } From 350c3d4b5c2480e13a2df07a2e29f06828fdcaeb Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:13:37 -0700 Subject: [PATCH 22/50] awk? --- e2e/bats/start.bash | 2 ++ 1 file changed, 2 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index a441407e44..f98595e4aa 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -34,6 +34,8 @@ teardown() { echo "xx 3" ps | grep [/[:space:]]replica | cut -d ' ' -f 1 echo "xx 4" + ps | grep [/[:space:]]replica | awk '{print $1}' + echo "xx 5" # find the replica that is the child of dfx. we do not have awk. REPLICA_PID=$(ps | grep [/[:space:]]replica | cut -d ' ' -f 1) From 5ded16610e93d4698ed469453190d72787752522 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:15:40 -0700 Subject: [PATCH 23/50] i guess we do have awk --- e2e/bats/start.bash | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index f98595e4aa..3f95ffde19 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -37,8 +37,7 @@ teardown() { ps | grep [/[:space:]]replica | awk '{print $1}' echo "xx 5" - # find the replica that is the child of dfx. we do not have awk. - REPLICA_PID=$(ps | grep [/[:space:]]replica | cut -d ' ' -f 1) + REPLICA_PID=$(ps | grep [/[:space:]]replica | awk '{print $1}') echo "replica pid is $REPLICA_PID" From a297b35827325e10cb2fee94d7bf06887efd409d Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:26:19 -0700 Subject: [PATCH 24/50] jobs? --- e2e/bats/start.bash | 3 +++ 1 file changed, 3 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 3f95ffde19..81f17669d9 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -25,6 +25,9 @@ teardown() { DFX_PID=$(cat .dfx/pid) + echo "jobs 0" + jobs -p + echo "xx 0" ps echo "xx 1" From 3f330091097e76f2647f4c098f19d268340864ec Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:30:40 -0700 Subject: [PATCH 25/50] ps aux, anything? --- e2e/bats/start.bash | 3 +++ 1 file changed, 3 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 81f17669d9..459fbb5c3a 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -28,6 +28,9 @@ teardown() { echo "jobs 0" jobs -p + echo "ps aux" + ps aux + echo "xx 0" ps echo "xx 1" From 1950a38d71a9a5a4a0b4a167a811b16dfbd4fc1e Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:33:16 -0700 Subject: [PATCH 26/50] force fail to see output --- e2e/bats/start.bash | 2 ++ 1 file changed, 2 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 459fbb5c3a..482683986c 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -61,5 +61,7 @@ teardown() { assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' + + assert_match "force fail" } From cf3e3d363a7138bf0056b5a508033c7a49410ef6 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:35:34 -0700 Subject: [PATCH 27/50] export --- e2e/bats/start.bash | 3 +++ 1 file changed, 3 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 482683986c..aae99be1e9 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -25,6 +25,9 @@ teardown() { DFX_PID=$(cat .dfx/pid) + echo "export" + export + echo "jobs 0" jobs -p From 9bbc394be924c5a25d7b13276024846ebd0a9d13 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:40:25 -0700 Subject: [PATCH 28/50] ps -T --- e2e/bats/start.bash | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index aae99be1e9..82e8122e65 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -34,7 +34,10 @@ teardown() { echo "ps aux" ps aux - echo "xx 0" + echo "ps -T" + ps -T + + echo "ps" ps echo "xx 1" ps | grep replica @@ -64,7 +67,5 @@ teardown() { assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' - - assert_match "force fail" } From 384aae7c46a0837015e236bda17c32bfa11c0286 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 11:46:28 -0700 Subject: [PATCH 29/50] remove ps -T --- e2e/bats/start.bash | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 82e8122e65..1afc9fa5fe 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -34,10 +34,7 @@ teardown() { echo "ps aux" ps aux - echo "ps -T" - ps -T - - echo "ps" + echo "ps" # on x86_64-darwin this fails ps echo "xx 1" ps | grep replica From d5072ff5d8472a1cc05ffc1d2e11ab78e33a929e Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 14:44:42 -0700 Subject: [PATCH 30/50] checkpoint (no segv on exit) --- .../actors/replica_webserver_coordinator.rs | 54 ++++++++++++++++++- src/dfx/src/actors/shutdown_controller.rs | 9 ++++ src/dfx/src/commands/start.rs | 8 ++- src/dfx/src/main.rs | 1 + 4 files changed, 69 insertions(+), 3 deletions(-) diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 7598839d34..78d6b64f52 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -6,15 +6,20 @@ use crate::lib::network::network_descriptor::NetworkDescriptor; use crate::lib::webserver::run_webserver; use actix::clock::{delay_for, Duration}; use actix::fut::wrap_future; -use actix::{Actor, Addr, AsyncContext, Context, Handler}; +use actix::{Actor, Addr, AsyncContext, Context, Handler, ResponseActFuture, WrapFuture, ActorContext, ActorFuture, Running, System, fut}; use actix_server::Server; use slog::{debug, error, info, Logger}; use std::net::SocketAddr; use std::path::PathBuf; +use crate::actors::shutdown_controller::signals::outbound::Shutdown; +use crate::actors::shutdown_controller::signals::ShutdownSubscribe; +use crate::actors::shutdown_controller::ShutdownController; +use actix_web::rt; pub struct Config { pub logger: Option, pub replica_addr: Addr, + pub shutdown_controller: Addr, pub bind: SocketAddr, pub serve_dir: PathBuf, pub providers: Vec, @@ -74,7 +79,22 @@ impl Actor for ReplicaWebserverCoordinator { self.config .replica_addr .do_send(PortReadySubscribe(ctx.address().recipient())); + self.config + .shutdown_controller + .do_send(ShutdownSubscribe(ctx.address().recipient::())); } + + // fn stopping(&mut self, _ctx: &mut Self::Context) -> Running { + // info!(self.logger, "Stopping the web server..."); + // if let Some(server) = self.server.take() { + // block_on(server.stop(true)); + // //System::current().block_on(wrap_future(server.stop(true))); + // } + // + // debug!(self.logger, "Stopped."); + // Running::Stop + // } + } impl Handler for ReplicaWebserverCoordinator { @@ -104,3 +124,35 @@ impl Handler for ReplicaWebserverCoordinator { } } } + +impl Handler for ReplicaWebserverCoordinator { + type Result = ResponseActFuture>; + + fn handle(&mut self, _msg: Shutdown, _ctx: &mut Self::Context) -> Self::Result { + if let Some(server) = self.server.take() { + eprintln!("stopping webserver"); + Box::pin( + server.stop(true) + .into_actor(self) // converts future to ActorFuture + .map(|_, _act, ctx| { + eprintln!("stopping ReplicaWebserverCoordinator"); + ctx.stop(); + eprintln!("stopped ReplicaWebserverCoordinator"); + Ok(()) + }), + ) + } else + { + Box::pin(fut::ok(()) + // async{} + // .into_actor(self) // converts future to ActorFuture + // .map(|_, _act, ctx| { + // eprintln!("stopping ReplicaWebserverCoordinator"); + // ctx.stop(); + // eprintln!("stopped ReplicaWebserverCoordinator"); + // Ok(()) + // }), + ) + } + } +} diff --git a/src/dfx/src/actors/shutdown_controller.rs b/src/dfx/src/actors/shutdown_controller.rs index 966e74c882..f0e5c49165 100644 --- a/src/dfx/src/actors/shutdown_controller.rs +++ b/src/dfx/src/actors/shutdown_controller.rs @@ -3,6 +3,7 @@ use crate::actors::shutdown_controller::signals::ShutdownTrigger; use actix::{Actor, Addr, AsyncContext, Context, Handler, Recipient}; use slog::Logger; use std::time::Duration; +use std::thread; pub mod signals { use actix::prelude::*; @@ -85,6 +86,13 @@ impl ShutdownController { }) .expect("Error setting Ctrl-C handler"); } + + fn shutdown_soon(&self, shutdown_controller: Addr) { + let _t = thread::spawn(move || { + std::thread::sleep(Duration::from_secs(10)); + shutdown_controller.do_send(ShutdownTrigger()); + }); + } } impl Actor for ShutdownController { @@ -92,6 +100,7 @@ impl Actor for ShutdownController { fn started(&mut self, ctx: &mut Self::Context) { self.install_ctrlc_handler(ctx.address()); + self.shutdown_soon(ctx.address()); } } diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index f9dcc99687..f30afc0450 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -162,10 +162,10 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { }) .start(); - let replica_addr = start_replica(env, &state_root, shutdown_controller)?; + let replica_addr = start_replica(env, &state_root, shutdown_controller.clone())?; let _webserver_coordinator = - start_webserver_coordinator(env, args, config, address_and_port, replica_addr)?; + start_webserver_coordinator(env, args, config, address_and_port, replica_addr, shutdown_controller)?; // Update the pid file. if let Ok(pid) = sysinfo::get_current_pid() { @@ -174,6 +174,8 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { system.run()?; + eprintln!("system.run() returned"); + Ok(()) } @@ -230,6 +232,7 @@ fn start_webserver_coordinator( config: Arc, address_and_port: SocketAddr, replica_addr: Addr, + shutdown_controller: Addr, ) -> DfxResult> { let network_descriptor = get_network_descriptor(env, args)?; let bootstrap_dir = env.get_cache().get_binary_command_path("bootstrap")?; @@ -243,6 +246,7 @@ fn start_webserver_coordinator( let coord_config = actors::replica_webserver_coordinator::Config { logger: Some(env.get_logger().clone()), replica_addr, + shutdown_controller, bind: address_and_port, serve_dir: bootstrap_dir, providers, diff --git a/src/dfx/src/main.rs b/src/dfx/src/main.rs index 7ba703f1a0..7aa7960873 100644 --- a/src/dfx/src/main.rs +++ b/src/dfx/src/main.rs @@ -180,6 +180,7 @@ fn main() { Err(e) => Err(e), }; + eprintln!("main about to exit"); if let Err(err) = result { eprintln!("{}", err); From 5a013367fe027719cef64855350cba3e1fa84e61 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 15:00:36 -0700 Subject: [PATCH 31/50] cleanup of webserver cleanup --- .../actors/replica_webserver_coordinator.rs | 43 ++++++------------- src/dfx/src/actors/shutdown_controller.rs | 2 +- src/dfx/src/commands/start.rs | 12 ++++-- src/dfx/src/main.rs | 1 - 4 files changed, 22 insertions(+), 36 deletions(-) diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 78d6b64f52..5880743654 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -1,20 +1,24 @@ use crate::actors::replica::signals::outbound::ReplicaReadySignal; use crate::actors::replica::signals::PortReadySubscribe; use crate::actors::replica::Replica; +use crate::actors::shutdown_controller::signals::outbound::Shutdown; +use crate::actors::shutdown_controller::signals::ShutdownSubscribe; +use crate::actors::shutdown_controller::ShutdownController; use crate::lib::error::DfxResult; use crate::lib::network::network_descriptor::NetworkDescriptor; use crate::lib::webserver::run_webserver; use actix::clock::{delay_for, Duration}; use actix::fut::wrap_future; -use actix::{Actor, Addr, AsyncContext, Context, Handler, ResponseActFuture, WrapFuture, ActorContext, ActorFuture, Running, System, fut}; +use actix::{ + Actor, Addr, AsyncContext, Context, Handler, + ResponseFuture, +}; use actix_server::Server; +use futures::future; +use futures::future::FutureExt; use slog::{debug, error, info, Logger}; use std::net::SocketAddr; use std::path::PathBuf; -use crate::actors::shutdown_controller::signals::outbound::Shutdown; -use crate::actors::shutdown_controller::signals::ShutdownSubscribe; -use crate::actors::shutdown_controller::ShutdownController; -use actix_web::rt; pub struct Config { pub logger: Option, @@ -94,7 +98,6 @@ impl Actor for ReplicaWebserverCoordinator { // debug!(self.logger, "Stopped."); // Running::Stop // } - } impl Handler for ReplicaWebserverCoordinator { @@ -126,33 +129,13 @@ impl Handler for ReplicaWebserverCoordinator { } impl Handler for ReplicaWebserverCoordinator { - type Result = ResponseActFuture>; + type Result = ResponseFuture>; fn handle(&mut self, _msg: Shutdown, _ctx: &mut Self::Context) -> Self::Result { if let Some(server) = self.server.take() { - eprintln!("stopping webserver"); - Box::pin( - server.stop(true) - .into_actor(self) // converts future to ActorFuture - .map(|_, _act, ctx| { - eprintln!("stopping ReplicaWebserverCoordinator"); - ctx.stop(); - eprintln!("stopped ReplicaWebserverCoordinator"); - Ok(()) - }), - ) - } else - { - Box::pin(fut::ok(()) - // async{} - // .into_actor(self) // converts future to ActorFuture - // .map(|_, _act, ctx| { - // eprintln!("stopping ReplicaWebserverCoordinator"); - // ctx.stop(); - // eprintln!("stopped ReplicaWebserverCoordinator"); - // Ok(()) - // }), - ) + Box::pin(server.stop(true).map(Ok)) + } else { + Box::pin(future::ok(())) } } } diff --git a/src/dfx/src/actors/shutdown_controller.rs b/src/dfx/src/actors/shutdown_controller.rs index f0e5c49165..353d6e8837 100644 --- a/src/dfx/src/actors/shutdown_controller.rs +++ b/src/dfx/src/actors/shutdown_controller.rs @@ -2,8 +2,8 @@ use crate::actors::shutdown_controller::signals::outbound::Shutdown; use crate::actors::shutdown_controller::signals::ShutdownTrigger; use actix::{Actor, Addr, AsyncContext, Context, Handler, Recipient}; use slog::Logger; -use std::time::Duration; use std::thread; +use std::time::Duration; pub mod signals { use actix::prelude::*; diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index f30afc0450..9c97c46b4f 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -164,8 +164,14 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { let replica_addr = start_replica(env, &state_root, shutdown_controller.clone())?; - let _webserver_coordinator = - start_webserver_coordinator(env, args, config, address_and_port, replica_addr, shutdown_controller)?; + let _webserver_coordinator = start_webserver_coordinator( + env, + args, + config, + address_and_port, + replica_addr, + shutdown_controller, + )?; // Update the pid file. if let Ok(pid) = sysinfo::get_current_pid() { @@ -174,8 +180,6 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { system.run()?; - eprintln!("system.run() returned"); - Ok(()) } diff --git a/src/dfx/src/main.rs b/src/dfx/src/main.rs index 7aa7960873..7ba703f1a0 100644 --- a/src/dfx/src/main.rs +++ b/src/dfx/src/main.rs @@ -180,7 +180,6 @@ fn main() { Err(e) => Err(e), }; - eprintln!("main about to exit"); if let Err(err) = result { eprintln!("{}", err); From 0bd5aba471c07e0ac5349111f3fe201619c40161 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 7 Oct 2020 16:34:43 -0700 Subject: [PATCH 32/50] comment out automatic stop --- src/dfx/src/actors/shutdown_controller.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/dfx/src/actors/shutdown_controller.rs b/src/dfx/src/actors/shutdown_controller.rs index 353d6e8837..6c5cec153f 100644 --- a/src/dfx/src/actors/shutdown_controller.rs +++ b/src/dfx/src/actors/shutdown_controller.rs @@ -2,7 +2,7 @@ use crate::actors::shutdown_controller::signals::outbound::Shutdown; use crate::actors::shutdown_controller::signals::ShutdownTrigger; use actix::{Actor, Addr, AsyncContext, Context, Handler, Recipient}; use slog::Logger; -use std::thread; +//use std::thread; use std::time::Duration; pub mod signals { @@ -87,12 +87,12 @@ impl ShutdownController { .expect("Error setting Ctrl-C handler"); } - fn shutdown_soon(&self, shutdown_controller: Addr) { - let _t = thread::spawn(move || { - std::thread::sleep(Duration::from_secs(10)); - shutdown_controller.do_send(ShutdownTrigger()); - }); - } + // fn shutdown_soon(&self, shutdown_controller: Addr) { + // let _t = thread::spawn(move || { + // std::thread::sleep(Duration::from_secs(10)); + // shutdown_controller.do_send(ShutdownTrigger()); + // }); + // } } impl Actor for ShutdownController { @@ -100,7 +100,7 @@ impl Actor for ShutdownController { fn started(&mut self, ctx: &mut Self::Context) { self.install_ctrlc_handler(ctx.address()); - self.shutdown_soon(ctx.address()); + //self.shutdown_soon(ctx.address()); } } From 3fcf0fe888bf43bce132fc98e773b8598799a3e3 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Thu, 8 Oct 2020 13:01:53 -0700 Subject: [PATCH 33/50] wait for dfx ping, not process, and remove sleep --- e2e/bats/start.bash | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 1afc9fa5fe..db9d11aa77 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -55,12 +55,11 @@ teardown() { echo "replica exited" - timeout $timeout sh -c \ - 'until ps | grep [/[:space:]]replica; do echo waiting for replica to restart; sleep 1; done' \ + timeout 15s sh -c \ + 'until dfx ping; do echo waiting for replica to restart; sleep 1; done' \ || (echo "replica did not restart" && ps aux && exit 1) echo "replica restarted" - sleep 10 assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' From cc5f7229e10161a0b5da33dc9d8548d1ad030e9f Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Thu, 8 Oct 2020 13:23:32 -0700 Subject: [PATCH 34/50] cleanup --- src/dfx/src/actors/replica_webserver_coordinator.rs | 11 ----------- src/dfx/src/actors/shutdown_controller.rs | 9 --------- src/dfx/src/commands/start.rs | 6 +----- 3 files changed, 1 insertion(+), 25 deletions(-) diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 5880743654..6092223d9d 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -87,17 +87,6 @@ impl Actor for ReplicaWebserverCoordinator { .shutdown_controller .do_send(ShutdownSubscribe(ctx.address().recipient::())); } - - // fn stopping(&mut self, _ctx: &mut Self::Context) -> Running { - // info!(self.logger, "Stopping the web server..."); - // if let Some(server) = self.server.take() { - // block_on(server.stop(true)); - // //System::current().block_on(wrap_future(server.stop(true))); - // } - // - // debug!(self.logger, "Stopped."); - // Running::Stop - // } } impl Handler for ReplicaWebserverCoordinator { diff --git a/src/dfx/src/actors/shutdown_controller.rs b/src/dfx/src/actors/shutdown_controller.rs index 6c5cec153f..966e74c882 100644 --- a/src/dfx/src/actors/shutdown_controller.rs +++ b/src/dfx/src/actors/shutdown_controller.rs @@ -2,7 +2,6 @@ use crate::actors::shutdown_controller::signals::outbound::Shutdown; use crate::actors::shutdown_controller::signals::ShutdownTrigger; use actix::{Actor, Addr, AsyncContext, Context, Handler, Recipient}; use slog::Logger; -//use std::thread; use std::time::Duration; pub mod signals { @@ -86,13 +85,6 @@ impl ShutdownController { }) .expect("Error setting Ctrl-C handler"); } - - // fn shutdown_soon(&self, shutdown_controller: Addr) { - // let _t = thread::spawn(move || { - // std::thread::sleep(Duration::from_secs(10)); - // shutdown_controller.do_send(ShutdownTrigger()); - // }); - // } } impl Actor for ShutdownController { @@ -100,7 +92,6 @@ impl Actor for ShutdownController { fn started(&mut self, ctx: &mut Self::Context) { self.install_ctrlc_handler(ctx.address()); - //self.shutdown_soon(ctx.address()); } } diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 9c97c46b4f..8f36116bf6 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -5,12 +5,12 @@ use crate::lib::message::UserMessage; use crate::lib::provider::get_network_descriptor; use crate::lib::replica_config::ReplicaConfig; use crate::util::get_reusable_socket_addr; - use crate::actors; use crate::actors::replica::Replica; use crate::actors::replica_webserver_coordinator::ReplicaWebserverCoordinator; use crate::actors::shutdown_controller; use crate::actors::shutdown_controller::ShutdownController; + use actix::{Actor, Addr}; use clap::{App, Arg, ArgMatches, SubCommand}; use delay::{Delay, Waiter}; @@ -151,10 +151,6 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { clean_state(temp_dir, &state_root)?; } - // We are doing this here to make sure we can write to the temp - // pid file. - std::fs::write(&pid_file_path, "")?; - let system = actix::System::new("dfx-start"); let shutdown_controller = ShutdownController::new(shutdown_controller::Config { From 5cd0dab5429e6f81518de8bbf342580975a35318 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Thu, 8 Oct 2020 14:39:47 -0700 Subject: [PATCH 35/50] remove hotwatch dependency --- Cargo.lock | 86 ---------------------------------------------- src/dfx/Cargo.toml | 1 - 2 files changed, 87 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c582742916..746c60f968 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1020,7 +1020,6 @@ dependencies = [ "flate2", "futures", "hex", - "hotwatch", "humanize-rs", "ic-agent", "ic-types", @@ -1317,25 +1316,6 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "05f8140122fa0d5dcb9fc8627cfce2b37cc1500f752636d46ea28bc26785c2f9" -[[package]] -name = "fsevent" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ab7d1bd1bd33cc98b0889831b72da23c0aa4df9cec7e0702f46ecea04b35db6" -dependencies = [ - "bitflags", - "fsevent-sys", -] - -[[package]] -name = "fsevent-sys" -version = "2.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f41b048a94555da0f42f1d632e2e19510084fb8e303b0daa2816e733fb3644a0" -dependencies = [ - "libc", -] - [[package]] name = "fuchsia-cprng" version = "0.1.1" @@ -1564,16 +1544,6 @@ dependencies = [ "winapi 0.3.9", ] -[[package]] -name = "hotwatch" -version = "0.4.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ba0add391e9cd7d19c29024617a44df79c867ab003bce7f3224c1636595ec740" -dependencies = [ - "log", - "notify", -] - [[package]] name = "http" version = "0.2.1" @@ -1761,26 +1731,6 @@ dependencies = [ "regex", ] -[[package]] -name = "inotify" -version = "0.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4816c66d2c8ae673df83366c18341538f234a26d65a9ecea5c348b453ac1d02f" -dependencies = [ - "bitflags", - "inotify-sys", - "libc", -] - -[[package]] -name = "inotify-sys" -version = "0.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e74a1aa87c59aeff6ef2cc2fa62d41bc43f54952f55652656b18a02fd5e356c0" -dependencies = [ - "libc", -] - [[package]] name = "instant" version = "0.1.7" @@ -1900,12 +1850,6 @@ version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" -[[package]] -name = "lazycell" -version = "1.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55" - [[package]] name = "leb128" version = "0.2.4" @@ -2065,18 +2009,6 @@ dependencies = [ "winapi 0.2.8", ] -[[package]] -name = "mio-extras" -version = "2.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52403fe290012ce777c4626790c8951324a2b9e3316b3143779c72b029742f19" -dependencies = [ - "lazycell", - "log", - "mio", - "slab", -] - [[package]] name = "mio-uds" version = "0.6.8" @@ -2209,24 +2141,6 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" -[[package]] -name = "notify" -version = "4.0.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "80ae4a7688d1fab81c5bf19c64fc8db920be8d519ce6336ed4e7efe024724dbd" -dependencies = [ - "bitflags", - "filetime", - "fsevent", - "fsevent-sys", - "inotify", - "libc", - "mio", - "mio-extras", - "walkdir", - "winapi 0.3.9", -] - [[package]] name = "num-bigint" version = "0.3.0" diff --git a/src/dfx/Cargo.toml b/src/dfx/Cargo.toml index 72494a93e8..c024488fea 100644 --- a/src/dfx/Cargo.toml +++ b/src/dfx/Cargo.toml @@ -37,7 +37,6 @@ indicatif = "0.13.0" lazy-init = "0.3.0" lazy_static = "1.4.0" libflate = "0.1.27" -hotwatch = "0.4.3" humanize-rs = "0.1.5" mockall = "0.6.0" net2 = "0.2.34" From 1a094d1eb4174d2a3f44dd5798cb861ea55f8809 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Thu, 8 Oct 2020 16:02:24 -0700 Subject: [PATCH 36/50] reorder exec for clarity --- .../actors/replica_webserver_coordinator.rs | 5 +-- src/dfx/src/commands/start.rs | 43 ++++++++----------- 2 files changed, 18 insertions(+), 30 deletions(-) diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 6092223d9d..2ca7e949ba 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -9,10 +9,7 @@ use crate::lib::network::network_descriptor::NetworkDescriptor; use crate::lib::webserver::run_webserver; use actix::clock::{delay_for, Duration}; use actix::fut::wrap_future; -use actix::{ - Actor, Addr, AsyncContext, Context, Handler, - ResponseFuture, -}; +use actix::{Actor, Addr, AsyncContext, Context, Handler, ResponseFuture}; use actix_server::Server; use futures::future; use futures::future::FutureExt; diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 8f36116bf6..554b883a9b 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -1,3 +1,8 @@ +use crate::actors; +use crate::actors::replica::Replica; +use crate::actors::replica_webserver_coordinator::ReplicaWebserverCoordinator; +use crate::actors::shutdown_controller; +use crate::actors::shutdown_controller::ShutdownController; use crate::config::dfinity::Config; use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; @@ -5,11 +10,6 @@ use crate::lib::message::UserMessage; use crate::lib::provider::get_network_descriptor; use crate::lib::replica_config::ReplicaConfig; use crate::util::get_reusable_socket_addr; -use crate::actors; -use crate::actors::replica::Replica; -use crate::actors::replica_webserver_coordinator::ReplicaWebserverCoordinator; -use crate::actors::shutdown_controller; -use crate::actors::shutdown_controller::ShutdownController; use actix::{Actor, Addr}; use clap::{App, Arg, ArgMatches, SubCommand}; @@ -109,7 +109,6 @@ fn fg_ping_and_wait(webserver_port_path: PathBuf, frontend_url: String) -> DfxRe ping_and_wait(&frontend_url_mod) } -// TODO(eftychis)/In progress: Rename to replica. /// Start the Internet Computer locally. Spawns a proxy to forward and /// manage browser requests. Responsible for running the network (one /// replica at the moment) and the proxy. @@ -119,37 +118,29 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { .ok_or(DfxError::CommandMustBeRunInAProject)?; let temp_dir = env.get_temp_dir(); - - let (frontend_url, address_and_port) = frontend_address(args, &config)?; + let pid_file_path = temp_dir.join("pid"); let webserver_port_path = temp_dir.join("webserver-port"); - std::fs::write(&webserver_port_path, "")?; - - // don't write to file since this arg means we send_background() - if !args.is_present("background") { - std::fs::write(&webserver_port_path, address_and_port.port().to_string())?; - } - - let temp_dir = env.get_temp_dir(); - let state_root = env.get_state_dir(); - let pid_file_path = temp_dir.join("pid"); check_previous_process_running(&pid_file_path)?; - // We are doing this here to make sure we can write to the temp - // pid file. - std::fs::write(&pid_file_path, "")?; + // As we know no start process is running in this project, we can + // clean up the state if it is necessary. + if args.is_present("clean") { + clean_state(temp_dir, &state_root)?; + } + + std::fs::write(&pid_file_path, "")?; // make sure we can write to this file + std::fs::write(&webserver_port_path, "")?; + + let (frontend_url, address_and_port) = frontend_address(args, &config)?; if args.is_present("background") { send_background()?; return fg_ping_and_wait(webserver_port_path, frontend_url); } - // As we know no start process is running in this project, we can - // clean up the state if it is necessary. - if args.is_present("clean") { - clean_state(temp_dir, &state_root)?; - } + std::fs::write(&webserver_port_path, address_and_port.port().to_string())?; let system = actix::System::new("dfx-start"); From dc074d565769aebcaba0f16ebbbd864dacce7002 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 9 Oct 2020 10:30:23 -0700 Subject: [PATCH 37/50] write_pid fn --- src/dfx/src/commands/start.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 554b883a9b..6127b87ae9 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -140,6 +140,7 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { return fg_ping_and_wait(webserver_port_path, frontend_url); } + write_pid(&pid_file_path); std::fs::write(&webserver_port_path, address_and_port.port().to_string())?; let system = actix::System::new("dfx-start"); @@ -160,11 +161,6 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { shutdown_controller, )?; - // Update the pid file. - if let Ok(pid) = sysinfo::get_current_pid() { - let _ = std::fs::write(&pid_file_path, pid.to_string()); - } - system.run()?; Ok(()) @@ -302,3 +298,9 @@ fn check_previous_process_running(dfx_pid_path: &Path) -> DfxResult<()> { } Ok(()) } + +fn write_pid(pid_file_path: &Path) { + if let Ok(pid) = sysinfo::get_current_pid() { + let _ = std::fs::write(&pid_file_path, pid.to_string()); + } +} From 6f521e59539a148df9ca3d985b6e3adce7c37de6 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 9 Oct 2020 12:51:20 -0700 Subject: [PATCH 38/50] move start_shutdown_controller to its own function; other refinements --- src/dfx/src/commands/start.rs | 60 ++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/src/dfx/src/commands/start.rs b/src/dfx/src/commands/start.rs index 6127b87ae9..dc620ed82c 100644 --- a/src/dfx/src/commands/start.rs +++ b/src/dfx/src/commands/start.rs @@ -7,6 +7,7 @@ use crate::config::dfinity::Config; use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; use crate::lib::message::UserMessage; +use crate::lib::network::network_descriptor::NetworkDescriptor; use crate::lib::provider::get_network_descriptor; use crate::lib::replica_config::ReplicaConfig; use crate::util::get_reusable_socket_addr; @@ -20,7 +21,6 @@ use std::io::Read; use std::net::SocketAddr; use std::path::{Path, PathBuf}; use std::process::Command; -use std::sync::Arc; use sysinfo::{System, SystemExt}; use tokio::runtime::Runtime; @@ -117,7 +117,10 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { .get_config() .ok_or(DfxError::CommandMustBeRunInAProject)?; + let network_descriptor = get_network_descriptor(env, args)?; + let temp_dir = env.get_temp_dir(); + let build_output_root = temp_dir.join(&network_descriptor.name).join("canisters"); let pid_file_path = temp_dir.join("pid"); let webserver_port_path = temp_dir.join("webserver-port"); let state_root = env.get_state_dir(); @@ -145,19 +148,16 @@ pub fn exec(env: &dyn Environment, args: &ArgMatches<'_>) -> DfxResult { let system = actix::System::new("dfx-start"); - let shutdown_controller = ShutdownController::new(shutdown_controller::Config { - logger: Some(env.get_logger().clone()), - }) - .start(); + let shutdown_controller = start_shutdown_controller(env)?; - let replica_addr = start_replica(env, &state_root, shutdown_controller.clone())?; + let replica = start_replica(env, &state_root, shutdown_controller.clone())?; let _webserver_coordinator = start_webserver_coordinator( env, - args, - config, + network_descriptor, address_and_port, - replica_addr, + build_output_root, + replica, shutdown_controller, )?; @@ -181,13 +181,20 @@ fn clean_state(temp_dir: &Path, state_root: &Path) -> DfxResult { Ok(()) } +fn start_shutdown_controller(env: &dyn Environment) -> DfxResult> { + let actor_config = shutdown_controller::Config { + logger: Some(env.get_logger().clone()), + }; + Ok(ShutdownController::new(actor_config).start()) +} + fn start_replica( env: &dyn Environment, state_root: &Path, shutdown_controller: Addr, ) -> DfxResult> { - let replica_pathbuf = env.get_cache().get_binary_command_path("replica")?; - let ic_starter_pathbuf = env.get_cache().get_binary_command_path("ic-starter")?; + let replica_path = env.get_cache().get_binary_command_path("replica")?; + let ic_starter_path = env.get_cache().get_binary_command_path("ic-starter")?; let temp_dir = env.get_temp_dir(); let client_configuration_dir = temp_dir.join("client-configuration"); @@ -203,44 +210,39 @@ fn start_replica( std::fs::write(&client_port_path, "")?; let replica_config = ReplicaConfig::new(state_root).with_random_port(&client_port_path); - Ok(actors::replica::Replica::new(actors::replica::Config { - ic_starter_path: ic_starter_pathbuf, + let actor_config = actors::replica::Config { + ic_starter_path, replica_config, - replica_path: replica_pathbuf, + replica_path, shutdown_controller, logger: Some(env.get_logger().clone()), - }) - .start()) + }; + Ok(actors::replica::Replica::new(actor_config).start()) } fn start_webserver_coordinator( env: &dyn Environment, - args: &ArgMatches<'_>, - config: Arc, - address_and_port: SocketAddr, + network_descriptor: NetworkDescriptor, + bind: SocketAddr, + build_output_root: PathBuf, replica_addr: Addr, shutdown_controller: Addr, ) -> DfxResult> { - let network_descriptor = get_network_descriptor(env, args)?; - let bootstrap_dir = env.get_cache().get_binary_command_path("bootstrap")?; + let serve_dir = env.get_cache().get_binary_command_path("bootstrap")?; // By default we reach to no external IC nodes. let providers = Vec::new(); - let build_output_root = config - .get_temp_path() - .join(network_descriptor.name.clone()) - .join("canisters"); - let coord_config = actors::replica_webserver_coordinator::Config { + let actor_config = actors::replica_webserver_coordinator::Config { logger: Some(env.get_logger().clone()), replica_addr, shutdown_controller, - bind: address_and_port, - serve_dir: bootstrap_dir, + bind, + serve_dir, providers, build_output_root, network_descriptor, }; - Ok(ReplicaWebserverCoordinator::new(coord_config).start()) + Ok(ReplicaWebserverCoordinator::new(actor_config).start()) } fn send_background() -> DfxResult<()> { From d67e138cbfd27c8217d483d37f04af14ab24bcd6 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 9 Oct 2020 13:40:43 -0700 Subject: [PATCH 39/50] Use [:space:] because I think \s is not working --- e2e/bats/utils/_.bash | 2 ++ e2e/bats/utils/assertions.bash | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/e2e/bats/utils/_.bash b/e2e/bats/utils/_.bash index 2306f26910..8fd46c78d7 100644 --- a/e2e/bats/utils/_.bash +++ b/e2e/bats/utils/_.bash @@ -70,6 +70,8 @@ dfx_start() { # Overwrite the default networks.local.bind 127.0.0.1:8000 with allocated port local webserver_port=$(cat .dfx/webserver-port) cat <<<$(jq .networks.local.bind=\"127.0.0.1:${webserver_port}\" dfx.json) >dfx.json + + ps | grep "[/[:space:]]dfx start" fi printf "Replica Configured Port: %s\n" "${port}" diff --git a/e2e/bats/utils/assertions.bash b/e2e/bats/utils/assertions.bash index 71878579de..0d726ea911 100644 --- a/e2e/bats/utils/assertions.bash +++ b/e2e/bats/utils/assertions.bash @@ -148,8 +148,8 @@ assert_process_exits() { # Asserts that `dfx start` and `replica` are no longer running assert_no_dfx_start_or_replica_processes() { - ! ( ps | grep "[/\s]dfx start" ) - ! ( ps | grep "[/\s]replica" ) + ! ( ps | grep "[/[:space:]]dfx start" ) + ! ( ps | grep "[/[:space:]]replica" ) } assert_file_eventually_exists() { From 37e2470e65e5eb7bf51124ebba865d9d5f21c7f4 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 9 Oct 2020 13:45:25 -0700 Subject: [PATCH 40/50] failing as expected --- e2e/bats/utils/_.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/bats/utils/_.bash b/e2e/bats/utils/_.bash index 8fd46c78d7..991589374f 100644 --- a/e2e/bats/utils/_.bash +++ b/e2e/bats/utils/_.bash @@ -71,7 +71,7 @@ dfx_start() { local webserver_port=$(cat .dfx/webserver-port) cat <<<$(jq .networks.local.bind=\"127.0.0.1:${webserver_port}\" dfx.json) >dfx.json - ps | grep "[/[:space:]]dfx start" + # ps | grep "[/[:space:]]dfx start" fi printf "Replica Configured Port: %s\n" "${port}" From 556bc5713c60b21b8dff12c7c160d1042373efd5 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 9 Oct 2020 14:14:32 -0700 Subject: [PATCH 41/50] clean up e2e test a bit --- e2e/bats/start.bash | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index db9d11aa77..6013b83eaf 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -34,17 +34,9 @@ teardown() { echo "ps aux" ps aux - echo "ps" # on x86_64-darwin this fails + echo "ps # this will fail :(" + # on x86_64-darwin this fails: ps - echo "xx 1" - ps | grep replica - echo "xx 2" - ps | grep [/[:space:]]replica - echo "xx 3" - ps | grep [/[:space:]]replica | cut -d ' ' -f 1 - echo "xx 4" - ps | grep [/[:space:]]replica | awk '{print $1}' - echo "xx 5" REPLICA_PID=$(ps | grep [/[:space:]]replica | awk '{print $1}') @@ -53,14 +45,10 @@ teardown() { kill -KILL $REPLICA_PID assert_process_exits $REPLICA_PID 15s - echo "replica exited" - timeout 15s sh -c \ 'until dfx ping; do echo waiting for replica to restart; sleep 1; done' \ || (echo "replica did not restart" && ps aux && exit 1) - echo "replica restarted" - assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' } From 95cd392b4d65cb2988e9b4e897cce083aabba3e3 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Fri, 9 Oct 2020 14:27:52 -0700 Subject: [PATCH 42/50] explain why we shut down the web server --- e2e/bats/utils/_.bash | 2 -- src/dfx/src/actors/replica_webserver_coordinator.rs | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/e2e/bats/utils/_.bash b/e2e/bats/utils/_.bash index 991589374f..2306f26910 100644 --- a/e2e/bats/utils/_.bash +++ b/e2e/bats/utils/_.bash @@ -70,8 +70,6 @@ dfx_start() { # Overwrite the default networks.local.bind 127.0.0.1:8000 with allocated port local webserver_port=$(cat .dfx/webserver-port) cat <<<$(jq .networks.local.bind=\"127.0.0.1:${webserver_port}\" dfx.json) >dfx.json - - # ps | grep "[/[:space:]]dfx start" fi printf "Replica Configured Port: %s\n" "${port}" diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 2ca7e949ba..3c0b10723e 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -119,6 +119,9 @@ impl Handler for ReplicaWebserverCoordinator { fn handle(&mut self, _msg: Shutdown, _ctx: &mut Self::Context) -> Self::Result { if let Some(server) = self.server.take() { + // We stop the webserver before shutting down because + // if we don't, the process will segfault + // while dropping actix stuff after main() returns. Box::pin(server.stop(true).map(Ok)) } else { Box::pin(future::ok(())) From f80e64cce2d65f6b5d48a02c5f451ff92832f7ad Mon Sep 17 00:00:00 2001 From: Stanley Jones <49165812+stanleygjones@users.noreply.github.com> Date: Wed, 14 Oct 2020 12:29:04 -0700 Subject: [PATCH 43/50] "Empty" commit to trigger Mergify check --- e2e/bats/start.bash | 1 + 1 file changed, 1 insertion(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 6013b83eaf..9be0b88479 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -1,5 +1,6 @@ #!/usr/bin/env bats + load utils/_ setup() { From f39033c4aec2d302545cac1f96f75f6583d7ba87 Mon Sep 17 00:00:00 2001 From: Eric Swanson <64809312+ericswanson-dfinity@users.noreply.github.com> Date: Wed, 14 Oct 2020 12:34:08 -0700 Subject: [PATCH 44/50] Undo extra line in e2e/bats/start.bash --- e2e/bats/start.bash | 2 -- 1 file changed, 2 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 9be0b88479..97d4362ca0 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -1,6 +1,5 @@ #!/usr/bin/env bats - load utils/_ setup() { @@ -53,4 +52,3 @@ teardown() { assert_command dfx canister call hello greet '("Omega")' assert_eq '("Hello, Omega!")' } - From 49844a338258b0c5da81032b0d222e8c1048b18a Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Mon, 19 Oct 2020 09:35:48 -0700 Subject: [PATCH 45/50] ps issue fixed? --- e2e/bats/start.bash | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 97d4362ca0..699cb309b8 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -34,8 +34,7 @@ teardown() { echo "ps aux" ps aux - echo "ps # this will fail :(" - # on x86_64-darwin this fails: + echo "ps # expected to no longer fail?" ps REPLICA_PID=$(ps | grep [/[:space:]]replica | awk '{print $1}') From f2cce31fc27ac10d38ffaa54e7f4b896427af2d9 Mon Sep 17 00:00:00 2001 From: Jude Taylor Date: Wed, 21 Oct 2020 12:27:46 -0700 Subject: [PATCH 46/50] use ps -x flag to match processes with no terminal --- e2e/bats/start.bash | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 699cb309b8..1d274527fd 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -31,13 +31,7 @@ teardown() { echo "jobs 0" jobs -p - echo "ps aux" - ps aux - - echo "ps # expected to no longer fail?" - ps - - REPLICA_PID=$(ps | grep [/[:space:]]replica | awk '{print $1}') + REPLICA_PID=$(ps x | grep [/[:space:]]replica | awk '{print $1}') echo "replica pid is $REPLICA_PID" From 0f446798926cf89a36bfb074eac2f610a9983aef Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 21 Oct 2020 13:41:44 -0700 Subject: [PATCH 47/50] see what ps x outputs --- e2e/bats/start.bash | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 1d274527fd..90e4466df4 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -28,8 +28,9 @@ teardown() { echo "export" export - echo "jobs 0" - jobs -p + echo "ps x" + ps x + assert_eq '("fail")' REPLICA_PID=$(ps x | grep [/[:space:]]replica | awk '{print $1}') From 030db00ffbe292cbc7a4bf580e6f3e684ac394e4 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Wed, 21 Oct 2020 14:00:31 -0700 Subject: [PATCH 48/50] clean up after ps x fix --- e2e/bats/start.bash | 9 --------- 1 file changed, 9 deletions(-) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 90e4466df4..4bd47afcc1 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -23,15 +23,6 @@ teardown() { assert_command dfx canister call hello greet '("Alpha")' assert_eq '("Hello, Alpha!")' - DFX_PID=$(cat .dfx/pid) - - echo "export" - export - - echo "ps x" - ps x - assert_eq '("fail")' - REPLICA_PID=$(ps x | grep [/[:space:]]replica | awk '{print $1}') echo "replica pid is $REPLICA_PID" From 0e6a0507f7c1b081fd6f772da9209ac89f52b463 Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Mon, 26 Oct 2020 14:48:56 -0700 Subject: [PATCH 49/50] note about non-robustness of using ps. --- e2e/bats/start.bash | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/e2e/bats/start.bash b/e2e/bats/start.bash index 4bd47afcc1..b51cfb7cfa 100644 --- a/e2e/bats/start.bash +++ b/e2e/bats/start.bash @@ -23,6 +23,15 @@ teardown() { assert_command dfx canister call hello greet '("Alpha")' assert_eq '("Hello, Alpha!")' + # ps "isn't a robust solution": https://dfinity.atlassian.net/browse/OPS-166 + # + # I guess we could make dfx write the replica pid somewhere. + # + # Anyway, if this test ends up sometimes killing a replica other than + # the one created by dfx for this test, then some other test might fail. + # + # Also, this does not work on linux: + # ps -o "ppid, pid, comm" REPLICA_PID=$(ps x | grep [/[:space:]]replica | awk '{print $1}') echo "replica pid is $REPLICA_PID" From a680e7e123c564fa89f2bc101ad502396196f10e Mon Sep 17 00:00:00 2001 From: Eric Swanson Date: Mon, 26 Oct 2020 15:11:32 -0700 Subject: [PATCH 50/50] client -> replica --- src/dfx/src/actors/replica_webserver_coordinator.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/dfx/src/actors/replica_webserver_coordinator.rs b/src/dfx/src/actors/replica_webserver_coordinator.rs index 3c0b10723e..9b5e23a587 100644 --- a/src/dfx/src/actors/replica_webserver_coordinator.rs +++ b/src/dfx/src/actors/replica_webserver_coordinator.rs @@ -52,14 +52,14 @@ impl ReplicaWebserverCoordinator { fn start_server(&self, port: u16) -> DfxResult { let mut providers = self.config.providers.clone(); - let ic_client_bind_addr = "http://localhost:".to_owned() + port.to_string().as_str(); - let ic_client_bind_addr = ic_client_bind_addr.as_str(); - let client_api_uri = - url::Url::parse(ic_client_bind_addr).expect("Failed to parse replica ingress url."); - providers.push(client_api_uri); + let ic_replica_bind_addr = "http://localhost:".to_owned() + port.to_string().as_str(); + let ic_replica_bind_addr = ic_replica_bind_addr.as_str(); + let replica_api_uri = + url::Url::parse(ic_replica_bind_addr).expect("Failed to parse replica ingress url."); + providers.push(replica_api_uri); info!( self.logger, - "Starting webserver on port {} for replica at {:?}", port, ic_client_bind_addr + "Starting webserver on port {} for replica at {:?}", port, ic_replica_bind_addr ); run_webserver(