From 274df3b8bb0484ddabb317160e962e3953d335d8 Mon Sep 17 00:00:00 2001 From: hashmap <hashmap@gmail.com> Date: Sat, 22 Sep 2018 09:34:28 +0200 Subject: [PATCH] Fix wallet APIs launch command (#1574) We used to launch a thread for API server inside the wallet crate, now we do it inside api crate, so the cmd tool launches API and exit. This fix makes sure that command will wait for API thread. --- api/src/handlers.rs | 2 +- api/src/rest.rs | 41 +++++++++++++++++------------- api/tests/rest.rs | 4 +-- wallet/src/libwallet/controller.rs | 39 ++++++++++++++++++---------- 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/api/src/handlers.rs b/api/src/handlers.rs index 37c4b6487..fbc589cb2 100644 --- a/api/src/handlers.rs +++ b/api/src/handlers.rs @@ -805,7 +805,7 @@ pub fn start_rest_apis( info!(LOGGER, "Starting HTTP API server at {}.", addr); let socket_addr: SocketAddr = addr.parse().expect("unable to parse socket address"); - apis.start(socket_addr, router) + apis.start(socket_addr, router).is_ok() } pub fn build_router( diff --git a/api/src/rest.rs b/api/src/rest.rs index 75d4d6aa6..1fe66b7fe 100644 --- a/api/src/rest.rs +++ b/api/src/rest.rs @@ -110,13 +110,19 @@ impl ApiServer { } /// Starts the ApiServer at the provided address. - pub fn start(&mut self, addr: SocketAddr, router: Router) -> bool { + pub fn start( + &mut self, + addr: SocketAddr, + router: Router, + ) -> Result<thread::JoinHandle<()>, Error> { if self.shutdown_sender.is_some() { - error!(LOGGER, "Can't start HTTP API server, it's running already"); - return false; + return Err(ErrorKind::Internal( + "Can't start HTTP API server, it's running already".to_string(), + ))?; } let (tx, _rx) = oneshot::channel::<()>(); - let _ = thread::Builder::new() + self.shutdown_sender = Some(tx); + thread::Builder::new() .name("apis".to_string()) .spawn(move || { let server = Server::bind(&addr) @@ -126,21 +132,24 @@ impl ApiServer { .map_err(|e| eprintln!("HTTP API server error: {}", e)); rt::run(server); - }); - - info!(LOGGER, "HTTP API server has been started"); - self.shutdown_sender = Some(tx); - true + }) + .map_err(|_| ErrorKind::Internal("failed to spawn API thread".to_string()).into()) } /// Starts the TLS ApiServer at the provided address. /// TODO support stop operation - pub fn start_tls(&mut self, addr: SocketAddr, router: Router, conf: TLSConfig) -> bool { + pub fn start_tls( + &mut self, + addr: SocketAddr, + router: Router, + conf: TLSConfig, + ) -> Result<thread::JoinHandle<()>, Error> { if self.shutdown_sender.is_some() { - error!(LOGGER, "Can't start HTTP API server, it's running already"); - return false; + return Err(ErrorKind::Internal( + "Can't start HTTPS API server, it's running already".to_string(), + ))?; } - let _ = thread::Builder::new() + thread::Builder::new() .name("apis".to_string()) .spawn(move || { let cert = Identity::from_pkcs12(conf.pkcs_bytes.as_slice(), &conf.pass).unwrap(); @@ -175,10 +184,8 @@ impl ApiServer { }); rt::run(server); - }); - - info!(LOGGER, "HTTPS API server has been started"); - true + }) + .map_err(|_| ErrorKind::Internal("failed to spawn API thread".to_string()).into()) } /// Stops the API server, it panics in case of error diff --git a/api/tests/rest.rs b/api/tests/rest.rs index b3447d426..944a43279 100644 --- a/api/tests/rest.rs +++ b/api/tests/rest.rs @@ -69,7 +69,7 @@ fn test_start_api() { router.add_middleware(counter.clone()); let server_addr = "127.0.0.1:14434"; let addr: SocketAddr = server_addr.parse().expect("unable to parse server address"); - assert!(server.start(addr, router)); + assert!(server.start(addr, router).is_ok()); let url = format!("http://{}/v1/", server_addr); let index = api::client::get::<Vec<String>>(url.as_str()).unwrap(); assert_eq!(index.len(), 2); @@ -94,7 +94,7 @@ fn test_start_api_tls() { let router = build_router(); let server_addr = "127.0.0.1:14444"; let addr: SocketAddr = server_addr.parse().expect("unable to parse server address"); - assert!(server.start_tls(addr, router, tls_conf)); + assert!(server.start_tls(addr, router, tls_conf).is_ok()); let url = format!("https://{}/v1/", server_addr); let index = api::client::get::<Vec<String>>(url.as_str()).unwrap(); assert_eq!(index.len(), 2); diff --git a/wallet/src/libwallet/controller.rs b/wallet/src/libwallet/controller.rs index 2ae2413f1..a84a9e755 100644 --- a/wallet/src/libwallet/controller.rs +++ b/wallet/src/libwallet/controller.rs @@ -16,18 +16,11 @@ //! invocations) as needed. //! Still experimental use api::{ApiServer, Handler, ResponseFuture, Router}; -use std::collections::HashMap; -use std::marker::PhantomData; -use std::net::SocketAddr; -use std::sync::{Arc, Mutex}; - +use core::core::Transaction; +use failure::ResultExt; use futures::future::{err, ok}; use futures::{Future, Stream}; use hyper::{Body, Request, Response, StatusCode}; -use serde::{Deserialize, Serialize}; -use serde_json; - -use core::core::Transaction; use keychain::Keychain; use libtx::slate::Slate; use libwallet::api::{APIForeign, APIOwner}; @@ -35,8 +28,13 @@ use libwallet::types::{ CbData, OutputData, SendTXArgs, TxLogEntry, WalletBackend, WalletClient, WalletInfo, }; use libwallet::{Error, ErrorKind}; +use serde::{Deserialize, Serialize}; +use serde_json; +use std::collections::HashMap; +use std::marker::PhantomData; +use std::net::SocketAddr; +use std::sync::{Arc, Mutex}; use url::form_urlencoded; - use util::secp::pedersen; use util::LOGGER; @@ -86,8 +84,14 @@ where let mut apis = ApiServer::new(); info!(LOGGER, "Starting HTTP Owner API server at {}.", addr); let socket_addr: SocketAddr = addr.parse().expect("unable to parse socket address"); - apis.start(socket_addr, router); - Ok(()) + let api_thread = apis + .start(socket_addr, router) + .context(ErrorKind::GenericError( + "API thread failed to start".to_string(), + ))?; + api_thread + .join() + .map_err(|e| ErrorKind::GenericError(format!("API thread paniced :{:?}", e)).into()) } /// Listener version, providing same API but listening for requests on a @@ -108,8 +112,15 @@ where let mut apis = ApiServer::new(); info!(LOGGER, "Starting HTTP Foreign API server at {}.", addr); let socket_addr: SocketAddr = addr.parse().expect("unable to parse socket address"); - apis.start(socket_addr, router); - Ok(()) + let api_thread = apis + .start(socket_addr, router) + .context(ErrorKind::GenericError( + "API thread failed to start".to_string(), + ))?; + + api_thread + .join() + .map_err(|e| ErrorKind::GenericError(format!("API thread paniced :{:?}", e)).into()) } type WalletResponseFuture = Box<Future<Item = Response<Body>, Error = Error> + Send>;