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

Cargo.toml

@@ -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]

parity-clib/Parity.java

@@ -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 void unsubscribeWebSocket(long session) {
 	}
 
 	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);

parity-clib/examples/cpp/main.cpp

@@ -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;

parity-clib/examples/java/Main.java

@@ -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 @@ public Callback(int type) {
 	}
 
 	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();
 	}
 

parity-clib/parity.h

@@ -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

parity-clib/src/java.rs

@@ -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,

parity-clib/src/lib.rs

@@ -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,

parity/configuration.rs

@@ -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),

parity/lib.rs

@@ -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,14 +223,21 @@ 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,
-			Rr: Fn() + '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);
 	for d in deprecated {
 		println!("{}", d);
 	}
 
-	execute(conf.into_command()?, on_client_rq, on_updater_rq)
+	execute(conf.into_command()?, logger, on_client_rq, on_updater_rq)
 }

parity/logger/src/lib.rs

@@ -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,
 	};