Core/grin
0
0
Fork 0
mirror of https://github.com/mimblewimble/grin.git synced 2025-01-21 11:31:08 +03:00
Code Issues Projects Releases Packages Wiki Activity Actions

Don't use process::exit in wallet cmd (#1913)

Browse source 
Unlike panic it doesn't call destructors (Drop) so all open transactions
are not closed. It's fine when LMDB file is open by the current process
only, but if another process keeps the same file open such transactions
will be considered alive until the second process exits. We usually have
one or more long-lived process (like `wallet listen`) which opens the
same wallet db as short-lived wallet commands. When a command fails it
calls process::exit and as result leaks a transaction.
This pr replaces such calls with an exit code return, which allows to call
all destructors before calling process::exit.
Fixes #1822
This commit is contained in:
hashmap 2018-11-04 21:26:46 +01:00 committed by GitHub
parent f645937a2b
commit 53bce41981
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 123 additions and 119 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
src/bin/cmd/client.rs
View file

@ -24,7 +24,7 @@ use servers::ServerConfig;
use term; use term;
use util::file::get_first_line; use util::file::get_first_line;
pub fn client_command(client_args: &ArgMatches, global_config: GlobalConfig) { pub fn client_command(client_args: &ArgMatches, global_config: GlobalConfig) -> i32 {
// just get defaults from the global config // just get defaults from the global config
let server_config = global_config.members.unwrap().server; let server_config = global_config.members.unwrap().server;
let api_secret = get_first_line(server_config.api_secret_path.clone()); let api_secret = get_first_line(server_config.api_secret_path.clone());
@ -56,6 +56,7 @@ pub fn client_command(client_args: &ArgMatches, global_config: GlobalConfig) {
} }
_ => panic!("Unknown client command, use 'grin help client' for details"), _ => panic!("Unknown client command, use 'grin help client' for details"),
} }
0
} }
pub fn show_status(config: &ServerConfig, api_secret: Option<String>) { pub fn show_status(config: &ServerConfig, api_secret: Option<String>) {

3
src/bin/cmd/server.rs
View file

@ -79,7 +79,7 @@ fn start_server_tui(config: servers::ServerConfig) {
/// stopping the Grin blockchain server. Processes all the command line /// stopping the Grin blockchain server. Processes all the command line
/// arguments to build a proper configuration and runs Grin with that /// arguments to build a proper configuration and runs Grin with that
/// configuration. /// configuration.
pub fn server_command(server_args: Option<&ArgMatches>, mut global_config: GlobalConfig) { pub fn server_command(server_args: Option<&ArgMatches>, mut global_config: GlobalConfig) -> i32 {
global::set_mining_mode( global::set_mining_mode(
global_config global_config
.members .members
@ -185,4 +185,5 @@ pub fn server_command(server_args: Option<&ArgMatches>, mut global_config: Globa
} else { } else {
start_server(server_config); start_server(server_config);
} }
0
} }

195
src/bin/cmd/wallet.rs
View file

@ -12,19 +12,17 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
use clap::ArgMatches;
use serde_json as json; use serde_json as json;
use std::fs::File; use std::fs::File;
use std::io::Read; use std::io::Read;
use std::path::{Path, PathBuf}; use std::path::{Path, PathBuf};
/// Wallet commands processing /// Wallet commands processing
use std::process::exit;
use std::sync::Arc; use std::sync::Arc;
use std::thread;
use std::time::Duration; use std::time::Duration;
use std::{process, thread};
use util::Mutex; use util::Mutex;
use clap::ArgMatches;
use api::TLSConfig; use api::TLSConfig;
use config::GlobalWalletConfig; use config::GlobalWalletConfig;
use core::{core, global}; use core::{core, global};
@ -70,14 +68,13 @@ pub fn instantiate_wallet(
db_wallet db_wallet
.set_parent_key_id_by_name(account) .set_parent_key_id_by_name(account)
.unwrap_or_else(|e| { .unwrap_or_else(|e| {
println!("Error starting wallet: {}", e); panic!("Error starting wallet: {}", e);
process::exit(0);
}); });
info!("Using LMDB Backend for wallet"); info!("Using LMDB Backend for wallet");
Box::new(db_wallet) Box::new(db_wallet)
} }
pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) { pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) -> i32 {
// just get defaults from the global config // just get defaults from the global config
let mut wallet_config = config.members.unwrap().wallet; let mut wallet_config = config.members.unwrap().wallet;
@ -121,18 +118,24 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
// give logging thread a moment to catch up // give logging thread a moment to catch up
thread::sleep(Duration::from_millis(200)); thread::sleep(Duration::from_millis(200));
// we are done here with creating the wallet, so just return // we are done here with creating the wallet, so just return
return; return 0;
} }
let passphrase = wallet_args.value_of("pass").unwrap_or_else(|| { let passphrase = match wallet_args.value_of("pass") {
None => {
error!("Failed to read passphrase."); error!("Failed to read passphrase.");
exit(1); return 1;
}); }
Some(p) => p,
};
let account = wallet_args.value_of("account").unwrap_or_else(|| { let account = match wallet_args.value_of("account") {
None => {
error!("Failed to read account."); error!("Failed to read account.");
exit(1); return 1;
}); }
Some(p) => p,
};
// Handle listener startup commands // Handle listener startup commands
{ {
@ -152,8 +155,7 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
.tls_certificate_key .tls_certificate_key
.clone() .clone()
.unwrap_or_else(|| { .unwrap_or_else(|| {
error!("Private key for certificate is not set"); panic!("Private key for certificate is not set");
exit(1);
}), }),
)), )),
}; };
@ -164,22 +166,20 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
} }
controller::foreign_listener(wallet, &wallet_config.api_listen_addr(), tls_conf) controller::foreign_listener(wallet, &wallet_config.api_listen_addr(), tls_conf)
.unwrap_or_else(|e| { .unwrap_or_else(|e| {
error!( panic!(
"Error creating wallet listener: {:?} Config: {:?}", "Error creating wallet listener: {:?} Config: {:?}",
e, wallet_config e, wallet_config
); );
exit(1);
}); });
} }
("owner_api", Some(_api_args)) => { ("owner_api", Some(_api_args)) => {
// TLS is disabled because we bind to localhost // TLS is disabled because we bind to localhost
controller::owner_listener(wallet, "127.0.0.1:13420", api_secret, None) controller::owner_listener(wallet, "127.0.0.1:13420", api_secret, None)
.unwrap_or_else(|e| { .unwrap_or_else(|e| {
error!( panic!(
"Error creating wallet api listener: {:?} Config: {:?}", "Error creating wallet api listener: {:?} Config: {:?}",
e, wallet_config e, wallet_config
); );
exit(1);
}); });
} }
("web", Some(_api_args)) => { ("web", Some(_api_args)) => {
@ -187,11 +187,10 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
start_webwallet_server(); start_webwallet_server();
controller::owner_listener(wallet, "127.0.0.1:13420", api_secret, tls_conf) controller::owner_listener(wallet, "127.0.0.1:13420", api_secret, tls_conf)
.unwrap_or_else(|e| { .unwrap_or_else(|e| {
error!( panic!(
"Error creating wallet api listener: {:?} Config: {:?}", "Error creating wallet api listener: {:?} Config: {:?}",
e, wallet_config e, wallet_config
); );
exit(1);
}); });
} }
_ => {} _ => {}
@ -217,9 +216,9 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
display::accounts(acct_mappings); display::accounts(acct_mappings);
Ok(()) Ok(())
}); });
if res.is_err() { if let Err(e) = res {
error!("Error listing accounts: {}", res.unwrap_err()); error!("Error listing accounts: {}", e);
exit(1); return Err(e);
} }
} else { } else {
let label = create.unwrap(); let label = create.unwrap();
@ -229,19 +228,18 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
println!("Account: '{}' Created!", label); println!("Account: '{}' Created!", label);
Ok(()) Ok(())
}); });
if res.is_err() { if let Err(e) = res {
thread::sleep(Duration::from_millis(200)); thread::sleep(Duration::from_millis(200));
println!("Error creating account '{}': {}", label, res.unwrap_err()); error!("Error creating account '{}': {}", label, e);
exit(1); return Err(e);
} }
} }
Ok(()) Ok(())
} }
("send", Some(send_args)) => { ("send", Some(send_args)) => {
let amount = send_args.value_of("amount").unwrap_or_else(|| { let amount = send_args.value_of("amount").ok_or_else(|| {
error!("Amount to send required"); ErrorKind::GenericError("Amount to send required".to_string())
exit(1); })?;
});
let amount = core::amount_from_hr_string(amount).map_err(|e| { let amount = core::amount_from_hr_string(amount).map_err(|e| {
ErrorKind::GenericError(format!( ErrorKind::GenericError(format!(
"Could not parse amount as a number with optional decimal point. e={:?}", "Could not parse amount as a number with optional decimal point. e={:?}",
@ -250,40 +248,38 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
})?; })?;
let minimum_confirmations: u64 = send_args let minimum_confirmations: u64 = send_args
.value_of("minimum_confirmations") .value_of("minimum_confirmations")
.unwrap_or_else(|| { .ok_or_else(|| {
error!("Minimum confirmations to send required"); ErrorKind::GenericError(
exit(1); "Minimum confirmations to send required".to_string(),
}).parse() )
.map_err(|e| { }).and_then(|v| {
v.parse().map_err(|e| {
ErrorKind::GenericError(format!( ErrorKind::GenericError(format!(
"Could not parse minimum_confirmations as a whole number. e={:?}", "Could not parse minimum_confirmations as a whole number. e={:?}",
e e
)) ))
})
})?; })?;
let selection_strategy = let selection_strategy =
send_args.value_of("selection_strategy").unwrap_or_else(|| { send_args.value_of("selection_strategy").ok_or_else(|| {
error!("Selection strategy required"); ErrorKind::GenericError("Selection strategy required".to_string())
exit(1); })?;
}); let method = send_args.value_of("method").ok_or_else(|| {
let method = send_args.value_of("method").unwrap_or_else(|| { ErrorKind::GenericError("Payment method required".to_string())
error!("Payment method required"); })?;
exit(1); let dest = send_args.value_of("dest").ok_or_else(|| {
}); ErrorKind::GenericError("Destination wallet address required".to_string())
let dest = send_args.value_of("dest").unwrap_or_else(|| { })?;
error!("Destination wallet address required");
exit(1);
});
let change_outputs = send_args let change_outputs = send_args
.value_of("change_outputs") .value_of("change_outputs")
.unwrap_or_else(|| { .ok_or_else(|| ErrorKind::GenericError("Change outputs required".to_string()))
error!("Change outputs required"); .and_then(|v| {
exit(1); v.parse().map_err(|e| {
}).parse()
.map_err(|e| {
ErrorKind::GenericError(format!( ErrorKind::GenericError(format!(
"Failed to parse number of change outputs. e={:?}", "Failed to parse number of change outputs. e={:?}",
e e
)) ))
})
})?; })?;
let fluff = send_args.is_present("fluff"); let fluff = send_args.is_present("fluff");
let max_outputs = 500; let max_outputs = 500;
@ -308,7 +304,7 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
s s
} }
Err(e) => { Err(e) => {
error!("Tx not created: {:?}", e); error!("Tx not created: {}", e);
match e.kind() { match e.kind() {
// user errors, don't backtrace // user errors, don't backtrace
libwallet::ErrorKind::NotEnoughFunds { .. } => {} libwallet::ErrorKind::NotEnoughFunds { .. } => {}
@ -319,7 +315,7 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
error!("Backtrace: {}", e.backtrace().unwrap()); error!("Backtrace: {}", e.backtrace().unwrap());
} }
}; };
exit(1); return Err(e);
} }
}; };
let result = api.post_tx(&slate, fluff); let result = api.post_tx(&slate, fluff);
@ -329,16 +325,15 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
Ok(()) Ok(())
} }
Err(e) => { Err(e) => {
error!("Tx not sent: {:?}", e); error!("Tx not sent: {}", e);
Err(e) Err(e)
} }
} }
} else { } else {
error!( return Err(ErrorKind::GenericError(format!(
"HTTP Destination should start with http://: or https://: {}", "HTTP Destination should start with http://: or https://: {}",
dest dest
); )).into());
exit(1);
} }
} else if method == "file" { } else if method == "file" {
api.send_tx( api.send_tx(
@ -352,26 +347,28 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
).map_err(|e| ErrorKind::GenericError(format!("Send failed. e={:?}", e)))?; ).map_err(|e| ErrorKind::GenericError(format!("Send failed. e={:?}", e)))?;
Ok(()) Ok(())
} else { } else {
error!("unsupported payment method: {}", method); return Err(ErrorKind::GenericError(format!(
exit(1); "unsupported payment method: {}",
method
)).into());
} }
} }
("receive", Some(send_args)) => { ("receive", Some(send_args)) => {
let mut receive_result: Result<(), grin_wallet::libwallet::Error> = Ok(()); let mut receive_result: Result<(), grin_wallet::libwallet::Error> = Ok(());
let tx_file = send_args.value_of("input").unwrap_or_else(|| { let tx_file = send_args.value_of("input").ok_or_else(|| {
error!("Transaction file required"); ErrorKind::GenericError("Transaction file required".to_string())
exit(1); })?;
});
if !Path::new(tx_file).is_file() { if !Path::new(tx_file).is_file() {
error!("File {} not found.", { tx_file }); return Err(
exit(1); ErrorKind::GenericError(format!("File {} not found.", tx_file)).into(),
);
} }
let res = controller::foreign_single_use(wallet, |api| { let res = controller::foreign_single_use(wallet, |api| {
receive_result = api.file_receive_tx(tx_file); receive_result = api.file_receive_tx(tx_file);
Ok(()) Ok(())
}); });
if res.is_err() { if res.is_err() {
exit(1); return res;
} else { } else {
info!( info!(
"Response file {}.response generated, sending it back to the transaction originator.", "Response file {}.response generated, sending it back to the transaction originator.",
@ -382,13 +379,13 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
} }
("finalize", Some(send_args)) => { ("finalize", Some(send_args)) => {
let fluff = send_args.is_present("fluff"); let fluff = send_args.is_present("fluff");
let tx_file = send_args.value_of("input").unwrap_or_else(|| { let tx_file = send_args.value_of("input").ok_or_else(|| {
error!("Receiver's transaction file required"); ErrorKind::GenericError("Receiver's transaction file required".to_string())
exit(1); })?;
});
if !Path::new(tx_file).is_file() { if !Path::new(tx_file).is_file() {
error!("File {} not found.", { tx_file }); return Err(
exit(1); ErrorKind::GenericError(format!("File {} not found.", tx_file)).into(),
);
} }
let mut pub_tx_f = File::open(tx_file)?; let mut pub_tx_f = File::open(tx_file)?;
let mut content = String::new(); let mut content = String::new();
@ -404,7 +401,7 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
Ok(()) Ok(())
} }
Err(e) => { Err(e) => {
error!("Tx not sent: {:?}", e); error!("Tx not sent: {}", e);
Err(e) Err(e)
} }
} }
@ -465,8 +462,9 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
Some(tx) => match tx.parse() { Some(tx) => match tx.parse() {
Ok(t) => Some(t), Ok(t) => Some(t),
Err(_) => { Err(_) => {
error!("Unable to parse argument 'id' as a number"); return Err(ErrorKind::GenericError(
exit(1); "Unable to parse argument 'id' as a number".to_string(),
).into());
} }
}, },
}; };
@ -508,16 +506,15 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
("repost", Some(repost_args)) => { ("repost", Some(repost_args)) => {
let tx_id = repost_args let tx_id = repost_args
.value_of("id") .value_of("id")
.unwrap_or_else(|| { .ok_or_else(|| {
error!("Transaction of a completed but unconfirmed transaction required (specify with --id=[id])"); ErrorKind::GenericError("Transaction of a completed but unconfirmed transaction required (specify with --id=[id])".to_string())
exit(1); }).and_then(|v|{
}) v.parse().map_err(|e| {
.parse().map_err(|e| {
ErrorKind::GenericError(format!( ErrorKind::GenericError(format!(
"Unable to parse argument 'id' as a number. e={:?}", "Unable to parse argument 'id' as a number. e={:?}",
e e
)) ))
})?; })})?;
let dump_file = repost_args.value_of("dumpfile"); let dump_file = repost_args.value_of("dumpfile");
let fluff = repost_args.is_present("fluff"); let fluff = repost_args.is_present("fluff");
@ -553,12 +550,15 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
("cancel", Some(tx_args)) => { ("cancel", Some(tx_args)) => {
let tx_id = tx_args let tx_id = tx_args
.value_of("id") .value_of("id")
.unwrap_or_else(|| { .ok_or_else(|| {
error!("'id' argument (-i) is required."); ErrorKind::GenericError("'id' argument (-i) is required.".to_string())
exit(1); }).and_then(|v| {
}).parse() v.parse().map_err(|e| {
.map_err(|e| { ErrorKind::GenericError(format!(
ErrorKind::GenericError(format!("Could not parse id parameter. e={:?}", e)) "Could not parse id parameter. e={:?}",
e
))
})
})?; })?;
let result = api.cancel_tx(tx_id); let result = api.cancel_tx(tx_id);
match result { match result {
@ -580,23 +580,26 @@ pub fn wallet_command(wallet_args: &ArgMatches, config: GlobalWalletConfig) {
Ok(()) Ok(())
} }
Err(e) => { Err(e) => {
error!("Wallet restore failed: {:?}", e); error!("Wallet restore failed: {}", e);
error!("Backtrace: {}", e.backtrace().unwrap()); error!("Backtrace: {}", e.backtrace().unwrap());
Err(e) Err(e)
} }
} }
} }
_ => { _ => {
error!("Unknown wallet command, use 'grin help wallet' for details"); return Err(ErrorKind::GenericError(
exit(1); "Unknown wallet command, use 'grin help wallet' for details".to_string(),
).into());
} }
} }
}); });
// we need to give log output a chance to catch up before exiting // we need to give log output a chance to catch up before exiting
thread::sleep(Duration::from_millis(100)); thread::sleep(Duration::from_millis(100));
if res.is_err() { if let Err(e) = res {
error!("Wallet command failed: {:?}", res); println!("Wallet command failed: {}", e);
exit(1); 1
} else {
0
} }
} }

21
src/bin/grin.rs
View file

@ -76,6 +76,11 @@ fn log_build_info() {
} }
fn main() { fn main() {
let exit_code = real_main();
std::process::exit(exit_code);
}
fn real_main() -> i32 {
let args = App::new("Grin") let args = App::new("Grin")
.version(crate_version!()) .version(crate_version!())
.author("The Grin Team") .author("The Grin Team")
@ -343,7 +348,7 @@ fn main() {
// If it's just a server config command, do it and exit // If it's just a server config command, do it and exit
if let ("config", Some(_)) = server_args.subcommand() { if let ("config", Some(_)) = server_args.subcommand() {
cmd::config_command_server(SERVER_CONFIG_FILE_NAME); cmd::config_command_server(SERVER_CONFIG_FILE_NAME);
return; return 0;
} }
} }
("wallet", Some(wallet_args)) => { ("wallet", Some(wallet_args)) => {
@ -410,24 +415,18 @@ fn main() {
match args.subcommand() { match args.subcommand() {
// server commands and options // server commands and options
("server", Some(server_args)) => { ("server", Some(server_args)) => {
cmd::server_command(Some(server_args), node_config.unwrap()); cmd::server_command(Some(server_args), node_config.unwrap())
} }
// client commands and options // client commands and options
("client", Some(client_args)) => { ("client", Some(client_args)) => cmd::client_command(client_args, node_config.unwrap()),
cmd::client_command(client_args, node_config.unwrap());
}
// client commands and options // client commands and options
("wallet", Some(wallet_args)) => { ("wallet", Some(wallet_args)) => cmd::wallet_command(wallet_args, wallet_config.unwrap()),
cmd::wallet_command(wallet_args, wallet_config.unwrap());
}
// If nothing is specified, try to just use the config file instead // If nothing is specified, try to just use the config file instead
// this could possibly become the way to configure most things // this could possibly become the way to configure most things
// with most command line options being phased out // with most command line options being phased out
_ => { _ => cmd::server_command(None, node_config.unwrap()),
cmd::server_command(None, node_config.unwrap());
}
} }
} }