prevent silent errors in daemon mode (#10007)

* prevent silent errors in daemon mode

* change author in Cargo.toml, add preamble to pipe.rs

* set the uid and gid on daemon process, fix permission errors when writing to pid file

* call setup_logger before daemonize to prevent crashing when attempting to create logfile

* map_err for calls to splice and ioctl, fix spaces in Cargo.toml

* split out daemonize to own repo

* removed util/daemonize

* renamed dep to parity-daemonize

* fix(parity-clib): enable `logger`

* bump parity-daemonize

* remove obsolete comment

Co-Authored-By: seunlanlege <seunlanlege@gmail.com>

* fix(grumbles): docs and log in ParityParams

* Add FIXME comment regarding @tomaka grumbles
* Unify logger with the C-API in ParityParams (less type-safety with more from_raw() conversions)
* Add better documentation in the `parity.h`

* Apply suggestions from code review

Co-Authored-By: seunlanlege <seunlanlege@gmail.com>

* docs(parity lib): add link to logging issue

* fix(parity-clib): JNI enable `logger`

* fix(parity-clib): update `Java example`

* Update example to the API changes
* Remove needless printouts which can be controlled via logger instead
This commit is contained in:
Seun LanLege 2019-02-01 19:31:02 +01:00 committed by Afri Schoedon
parent 12c42bce9b
commit 0f9b2218da
13 changed files with 162 additions and 70 deletions

24
Cargo.lock generated
View File

@ -503,14 +503,6 @@ dependencies = [
"winapi 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "daemonize"
version = "0.3.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "difference"
version = "1.0.0"
@ -2419,6 +2411,18 @@ dependencies = [
"tiny-keccak 1.4.2 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "parity-daemonize"
version = "0.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
dependencies = [
"ansi_term 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)",
"failure 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)",
"libc 0.2.43 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)",
"mio 0.6.16 (registry+https://github.com/rust-lang/crates.io-index)",
]
[[package]]
name = "parity-ethereum"
version = "2.4.0"
@ -2430,7 +2434,6 @@ dependencies = [
"cli-signer 1.4.0",
"common-types 0.1.0",
"ctrlc 1.1.1 (git+https://github.com/paritytech/rust-ctrlc.git)",
"daemonize 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"dir 0.1.2",
"docopt 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)",
"ethcore 1.12.0",
@ -2465,6 +2468,7 @@ dependencies = [
"number_prefix 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
"panic_hook 0.1.0",
"parity-bytes 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
"parity-daemonize 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"parity-hash-fetch 1.12.0",
"parity-ipfs-api 1.12.0",
"parity-local-store 0.1.0",
@ -4460,7 +4464,6 @@ dependencies = [
"checksum csv-core 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "4dd8e6d86f7ba48b4276ef1317edc8cc36167546d8972feb4a2b5fec0b374105"
"checksum ct-logs 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "95a4bf5107667e12bf6ce31a3a5066d67acc88942b6742117a41198734aaccaa"
"checksum ctrlc 1.1.1 (git+https://github.com/paritytech/rust-ctrlc.git)" = "<none>"
"checksum daemonize 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4093d27eb267d617f03c2ee25d4c3ca525b89a76154001954a11984508ffbde5"
"checksum difference 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b3304d19798a8e067e48d8e69b2c37f0b5e9b4e462504ad9e27e9f3fce02bba8"
"checksum digest 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)" = "03b072242a8cbaf9c145665af9d250c59af3b958f83ed6824e13533cf76d5b90"
"checksum docopt 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "db2906c2579b5b7207fc1e328796a9a8835dc44e22dbe8e460b1d636f9a7b225"
@ -4588,6 +4591,7 @@ dependencies = [
"checksum owning_ref 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "cdf84f41639e037b484f93433aa3897863b561ed65c6e59c7073d7c561710f37"
"checksum parity-bytes 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "fa5168b4cf41f3835e4bc6ffb32f51bc9365dc50cb351904595b3931d917fd0c"
"checksum parity-crypto 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "8adf489acb31f1922db0ce43803b6f48a425241a8473611be3cc625a8e4a4c47"
"checksum parity-daemonize 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "c6dbaae957a3ddd307fe0ea0361251ceb651a19810b60d404080b258384da292"
"checksum parity-path 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)" = "5962540f99d3895d9addf535f37ab1397886bc2c68e59efd040ef458e5f8c3f7"
"checksum parity-rocksdb 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "cd55d2d6d6000ec99f021cf52c9acc7d2a402e14f95ced4c5de230696fabe00b"
"checksum parity-rocksdb-sys 0.5.5 (registry+https://github.com/rust-lang/crates.io-index)" = "5bbb241262c768522f6460f0e2672dee185c8504d4d0a5a5bab45c1147981c4f"

View File

@ -84,7 +84,7 @@ fake-fetch = { path = "util/fake-fetch" }
winapi = { version = "0.3.4", features = ["winsock2", "winuser", "shellapi"] }
[target.'cfg(not(windows))'.dependencies]
daemonize = "0.3"
parity-daemonize = "0.1.1"
[features]
miner-debug = ["ethcore/miner-debug"]
@ -133,6 +133,10 @@ members = [
"evmbin",
"parity-clib",
"whisper/cli",
"util/triehash-ethereum",
"util/keccak-hasher",
"util/patricia-trie-ethereum",
"util/fastmap"
]
[patch.crates-io]

View File

@ -28,9 +28,9 @@ public class Parity {
*
* @param options The CLI options to start Parity with
*/
public Parity(String[] options) {
public Parity(String[] options, String loggerMode, String loggerFile) {
long config = configFromCli(options);
inner = build(config);
inner = build(config, loggerMode, loggerFile);
}
/** Performs an asynchronous RPC query by spawning a background thread that is executed until
@ -76,7 +76,7 @@ public class Parity {
}
private static native long configFromCli(String[] cliOptions);
private static native long build(long config);
private static native long build(long config, String loggerMode, String loggerFile);
private static native void destroy(long inner);
private static native void rpcQueryNative(long inner, String rpc, long timeoutMillis, Object callback);
private static native long subscribeWebSocketNative(long inner, String rpc, Object callback);

View File

@ -53,10 +53,8 @@ const std::vector<std::string> ws_subscriptions {
void callback(void* user_data, const char* response, size_t _len) {
Callback* cb = static_cast<Callback*>(user_data);
if (cb->type == CALLBACK_RPC) {
printf("rpc response: %s\r\n", response);
cb->counter -= 1;
} else if (cb->type == CALLBACK_WS) {
printf("websocket response: %s\r\n", response);
std::regex is_subscription ("\\{\"jsonrpc\":\"2.0\",\"result\":\"0[xX][a-fA-F0-9]{16}\",\"id\":1\\}");
if (std::regex_match(response, is_subscription) == true) {
cb->counter -= 1;
@ -153,7 +151,8 @@ void* parity_run(std::vector<const char*> args) {
ParityParams cfg = {
.configuration = nullptr,
.on_client_restart_cb = callback,
.on_client_restart_cb_custom = nullptr
.on_client_restart_cb_custom = nullptr,
.logger = nullptr
};
std::vector<size_t> str_lens;
@ -173,6 +172,10 @@ void* parity_run(std::vector<const char*> args) {
}
}
// enable logging but only the `rpc module` and don't write it to a file
char log_mode [] = "rpc=trace";
parity_set_logger(log_mode, strlen(log_mode), nullptr, 0, &cfg.logger);
void *parity = nullptr;
if (parity_start(&cfg, &parity) != 0) {
return nullptr;

View File

@ -35,7 +35,9 @@ class Main {
};
public static void runParity(String[] config) {
Parity parity = new Parity(config);
String loggerMode = "rpc=trace";
String loggerFile = "foo.log";
Parity parity = new Parity(config, loggerMode, loggerFile);
Callback rpcCallback = new Callback(1);
Callback webSocketCallback = new Callback(2);
@ -94,12 +96,6 @@ class Callback {
}
public void callback(Object response) {
response = (String) response;
if (callbackType == 1) {
System.out.println("rpc: " + response);
} else if (callbackType == 2) {
System.out.println("ws: " + response);
}
counter.getAndIncrement();
}

View File

@ -35,6 +35,9 @@ struct ParityParams {
/// Custom parameter passed to the `on_client_restart_cb` callback as first parameter.
void *on_client_restart_cb_custom;
/// Logger object which must be created by the `parity_config_logger` function
void *logger;
};
#ifdef __cplusplus
@ -63,6 +66,33 @@ extern "C" {
///
int parity_config_from_cli(char const* const* args, size_t const* arg_lens, size_t len, void** out);
/// Builds a new logger object which should be a member of the `ParityParams struct`
///
/// - log_mode : String representing the log mode according to `Rust LOG` or nullptr to disable logging.
/// See module documentation for `ethcore-logger` for more info.
/// - log_mode_len : Length of the log_mode or zero to disable logging
/// - log_file : String respresenting the file name to write to log to or nullptr to disable logging to a file
/// - log_mode_len : Length of the log_file or zero to disable logging to a file
/// - logger : Pointer to point to the created `Logger` object
/// **Important**: This function must only be called exactly once otherwise it will panic. If you want to disable a
/// logging mode or logging to a file make sure that you pass the `length` as zero
///
/// # Example
///
/// ```no_run
/// void* cfg;
/// const char *args[] = {"--light", "--can-restart"};
/// size_t str_lens[] = {7, 13};
/// if (parity_config_from_cli(args, str_lens, 2, &cfg) != 0) {
/// return 1;
/// }
/// char[] logger_mode = "rpc=trace";
/// parity_set_logger(logger_mode, strlen(logger_mode), nullptr, 0, &cfg.logger);
/// ```
///
int parity_set_logger(const char* log_mode, size_t log_mode_len, const char* log_file, size_t log_file_len, void** logger);
/// Destroys a configuration object created earlier.
///
/// **Important**: You probably don't need to call this function. Calling `parity_start` destroys

View File

@ -20,7 +20,7 @@ use std::time::Duration;
use std::thread;
use std::os::raw::c_void;
use {parity_config_from_cli, parity_destroy, parity_start, parity_unsubscribe_ws, ParityParams, error};
use {parity_config_from_cli, parity_destroy, parity_set_logger, parity_start, parity_unsubscribe_ws, ParityParams, error};
use futures::{Future, Stream};
use futures::sync::mpsc;
@ -96,12 +96,24 @@ pub unsafe extern "system" fn Java_io_parity_ethereum_Parity_configFromCli(env:
}
#[no_mangle]
pub unsafe extern "system" fn Java_io_parity_ethereum_Parity_build(env: JNIEnv, _: JClass, config: va_list) -> jlong {
let params = ParityParams {
pub unsafe extern "system" fn Java_io_parity_ethereum_Parity_build(
env: JNIEnv,
_: JClass,
config: va_list,
logger_mode: JString,
logger_file: JString
) -> jlong {
let mut params = ParityParams {
configuration: config,
.. mem::zeroed()
};
let logger_mode: String = env.get_string(logger_mode).expect("valid JString; qed").into();
let logger_file: String = env.get_string(logger_file).expect("valid JString; qed").into();
parity_set_logger(logger_mode.as_ptr(), logger_mode.as_bytes().len(), logger_file.as_ptr(),
logger_file.as_bytes().len(), &mut params.logger);
let mut out = ptr::null_mut();
match parity_start(&params, &mut out) {
0 => out as jlong,

View File

@ -54,6 +54,7 @@ pub struct ParityParams {
pub configuration: *mut c_void,
pub on_client_restart_cb: Callback,
pub on_client_restart_cb_custom: *mut c_void,
pub logger: *mut c_void
}
#[no_mangle]
@ -112,6 +113,7 @@ pub unsafe extern fn parity_start(cfg: *const ParityParams, output: *mut *mut c_
*output = ptr::null_mut();
let cfg: &ParityParams = &*cfg;
let logger = Arc::from_raw(cfg.logger as *mut parity_ethereum::RotatingLogger);
let config = Box::from_raw(cfg.configuration as *mut parity_ethereum::Configuration);
let on_client_restart_cb = {
@ -122,7 +124,7 @@ pub unsafe extern fn parity_start(cfg: *const ParityParams, output: *mut *mut c_
move |new_chain: String| { cb.call(new_chain.as_bytes()); }
};
let action = match parity_ethereum::start(*config, on_client_restart_cb, || {}) {
let action = match parity_ethereum::start(*config, logger, on_client_restart_cb, || {}) {
Ok(action) => action,
Err(_) => return 1,
};
@ -262,6 +264,25 @@ pub unsafe extern fn parity_set_panic_hook(callback: Callback, param: *mut c_voi
});
}
#[no_mangle]
pub unsafe extern fn parity_set_logger(
logger_mode: *const u8,
logger_mode_len: usize,
log_file: *const u8,
log_file_len: usize,
logger: *mut *mut c_void) {
let mut logger_cfg = parity_ethereum::LoggerConfig::default();
logger_cfg.mode = String::from_utf8(slice::from_raw_parts(logger_mode, logger_mode_len).to_owned()).ok();
// Make sure an empty string is not constructed as file name (to prevent panic)
if log_file_len != 0 && !log_file.is_null() {
logger_cfg.file = String::from_utf8(slice::from_raw_parts(log_file, log_file_len).to_owned()).ok();
}
*logger = Arc::into_raw(parity_ethereum::setup_log(&logger_cfg).expect("Logger initialized only once; qed")) as *mut _;
}
// Internal structure for handling callbacks that get passed a string.
struct CallbackStr {
user_data: *mut c_void,

View File

@ -473,7 +473,8 @@ impl Configuration {
}
}
fn logger_config(&self) -> LogConfig {
/// returns logger config
pub fn logger_config(&self) -> LogConfig {
LogConfig {
mode: self.args.arg_logging.clone(),
color: !self.args.flag_no_color && !cfg!(windows),

View File

@ -111,19 +111,22 @@ mod user_defaults;
mod whisper;
mod db;
use std::io::BufReader;
use std::fs::File;
use hash::keccak_buffer;
use std::io::BufReader;
use std::sync::Arc;
use cli::Args;
use configuration::{Cmd, Execute};
use deprecated::find_deprecated;
use ethcore_logger::setup_log;
use hash::keccak_buffer;
#[cfg(feature = "memory_profiling")]
use std::alloc::System;
pub use self::configuration::Configuration;
pub use self::run::RunningClient;
pub use parity_rpc::PubSubSession;
pub use ethcore_logger::{Config as LoggerConfig, setup_log, RotatingLogger};
#[cfg(feature = "memory_profiling")]
#[global_allocator]
@ -180,14 +183,13 @@ pub enum ExecutionAction {
Running(RunningClient),
}
fn execute<Cr, Rr>(command: Execute, on_client_rq: Cr, on_updater_rq: Rr) -> Result<ExecutionAction, String>
fn execute<Cr, Rr>(
command: Execute,
logger: Arc<RotatingLogger>,
on_client_rq: Cr, on_updater_rq: Rr) -> Result<ExecutionAction, String>
where Cr: Fn(String) + 'static + Send,
Rr: Fn() + 'static + Send
{
// TODO: move this to `main()` and expose in the C API so that users can setup logging the way
// they want
let logger = setup_log(&command.logger).expect("Logger is initialized only once; qed");
#[cfg(feature = "deadlock_detection")]
run_deadlock_detection_thread();
@ -221,8 +223,15 @@ fn execute<Cr, Rr>(command: Execute, on_client_rq: Cr, on_updater_rq: Rr) -> Res
/// binary.
///
/// On error, returns what to print on stderr.
pub fn start<Cr, Rr>(conf: Configuration, on_client_rq: Cr, on_updater_rq: Rr) -> Result<ExecutionAction, String>
where Cr: Fn(String) + 'static + Send,
// FIXME: totally independent logging capability, see https://github.com/paritytech/parity-ethereum/issues/10252
pub fn start<Cr, Rr>(
conf: Configuration,
logger: Arc<RotatingLogger>,
on_client_rq: Cr,
on_updater_rq: Rr
) -> Result<ExecutionAction, String>
where
Cr: Fn(String) + 'static + Send,
Rr: Fn() + 'static + Send
{
let deprecated = find_deprecated(&conf.args);
@ -230,5 +239,5 @@ pub fn start<Cr, Rr>(conf: Configuration, on_client_rq: Cr, on_updater_rq: Rr) -
println!("{}", d);
}
execute(conf.into_command()?, on_client_rq, on_updater_rq)
execute(conf.into_command()?, logger, on_client_rq, on_updater_rq)
}

View File

@ -94,7 +94,7 @@ pub fn setup_log(config: &Config) -> Result<Arc<RotatingLogger>, String> {
let maybe_file = match config.file.as_ref() {
Some(f) => Some(open_options
.append(true).create(true).open(f)
.map_err(|_| format!("Cannot write to log file given: {}", f))?),
.map_err(|e| format!("Cannot write to log file given: {}, {}", f, e))?),
None => None,
};

View File

@ -26,8 +26,11 @@ extern crate log;
extern crate panic_hook;
extern crate parity_ethereum;
extern crate parking_lot;
extern crate parity_daemonize;
extern crate ansi_term;
#[cfg(windows)] extern crate winapi;
extern crate ethcore_logger;
use std::ffi::OsString;
use std::fs::{remove_file, metadata, File, create_dir_all};
@ -36,12 +39,13 @@ use std::path::PathBuf;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::{process, env};
use ansi_term::Colour;
use ctrlc::CtrlC;
use dir::default_hypervisor_path;
use fdlimit::raise_fd_limit;
use parity_ethereum::{start, ExecutionAction};
use parking_lot::{Condvar, Mutex};
use ethcore_logger::setup_log;
const PLEASE_RESTART_EXIT_CODE: i32 = 69;
const PARITY_EXECUTABLE_NAME: &str = "parity";
@ -184,11 +188,33 @@ fn main_direct(force_can_restart: bool) -> i32 {
parity_ethereum::Configuration::parse_cli(&args).unwrap_or_else(|e| e.exit())
};
let logger = setup_log(&conf.logger_config()).expect("Logger is initialized only once; qed");
if let Some(spec_override) = take_spec_name_override() {
conf.args.flag_testnet = false;
conf.args.arg_chain = spec_override;
}
let handle = if let Some(ref pid) = conf.args.arg_daemon_pid_file {
info!("{}", Colour::Blue.paint("starting in daemon mode").to_string());
let _ = std::io::stdout().flush();
match parity_daemonize::daemonize(pid) {
Ok(h) => Some(h),
Err(e) => {
error!(
"{}",
Colour::Red.paint(format!("{}", e))
);
// flush before returning
let _ = std::io::stderr().flush();
return 1;
}
}
} else {
None
};
let can_restart = force_can_restart || conf.args.flag_can_restart;
// increase max number of open files
@ -208,6 +234,7 @@ fn main_direct(force_can_restart: bool) -> i32 {
let exec = if can_restart {
start(
conf,
logger,
{
let e = exit.clone();
let exiting = exiting.clone();
@ -239,10 +266,9 @@ fn main_direct(force_can_restart: bool) -> i32 {
}
}
)
} else {
trace!(target: "mode", "Not hypervised: not setting exit handlers.");
start(conf, move |_| {}, move || {})
start(conf, logger, move |_| {}, move || {})
};
let res = match exec {
@ -283,6 +309,12 @@ fn main_direct(force_can_restart: bool) -> i32 {
}
});
// so the client has started successfully
// if this is a daemon, detach from the parent process
if let Some(mut handle) = handle {
handle.detach()
}
// Wait for signal
let mut lock = exit.0.lock();
if !lock.should_exit {
@ -306,6 +338,11 @@ fn main_direct(force_can_restart: bool) -> i32 {
},
},
Err(err) => {
// error occured during start up
// if this is a daemon, detach from the parent process
if let Some(mut handle) = handle {
handle.detach_with_msg(format!("{}", Colour::Red.paint(&err)))
}
writeln!(&mut stdio::stderr(), "{}", err).expect("StdErr available; qed");
1
},

View File

@ -488,14 +488,6 @@ fn execute_impl<Cr, Rr>(cmd: RunCmd, logger: Arc<RotatingLogger>, on_client_rq:
let passwords = passwords_from_files(&cmd.acc_conf.password_files)?;
// Run in daemon mode.
// Note, that it should be called before we leave any file descriptor open,
// since `daemonize` will close them.
if let Some(pid_file) = cmd.daemon {
info!("Running as a daemon process!");
daemonize(pid_file)?;
}
// prepare account provider
let account_provider = Arc::new(prepare_account_provider(&cmd.spec, &cmd.dirs, &spec.data_dir, cmd.acc_conf, &passwords)?);
@ -954,23 +946,6 @@ pub fn execute<Cr, Rr>(cmd: RunCmd, logger: Arc<RotatingLogger>,
}
}
#[cfg(not(windows))]
fn daemonize(pid_file: String) -> Result<(), String> {
extern crate daemonize;
daemonize::Daemonize::new()
.pid_file(pid_file)
.chown_pid_file(true)
.start()
.map(|_| ())
.map_err(|e| format!("Couldn't daemonize; {}", e))
}
#[cfg(windows)]
fn daemonize(_pid_file: String) -> Result<(), String> {
Err("daemon is no supported on windows".into())
}
fn print_running_environment(data_dir: &str, dirs: &Directories, db_dirs: &DatabaseDirectories) {
info!("Starting {}", Colour::White.bold().paint(version()));
info!("Keys path {}", Colour::White.bold().paint(dirs.keys_path(data_dir).to_string_lossy().into_owned()));