From 450ae4147fa9c6cebf7f10eba70960aa9e610ff2 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Sun, 13 Mar 2016 13:03:02 +0100 Subject: [PATCH 1/4] memory and expiration mngmt --- util/src/keys/store.rs | 57 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/util/src/keys/store.rs b/util/src/keys/store.rs index 610dda9f5..3477130e9 100644 --- a/util/src/keys/store.rs +++ b/util/src/keys/store.rs @@ -135,6 +135,19 @@ impl AccountService { secret_store: secret_store } } + + #[cfg(test)] + fn new_test(temp: &::devtools::RandomTempPath) -> Self { + let secret_store = RwLock::new(SecretStore::new_test(temp)); + AccountService { + secret_store: secret_store + } + } + + /// Ticks the account service + pub fn tick(&self) { + self.secret_store.write().unwrap().collect_garbage(); + } } impl Default for SecretStore { @@ -256,6 +269,17 @@ impl SecretStore { let unlock = try!(read_lock.get(account).ok_or(SigningError::AccountNotUnlocked)); Ok(unlock.secret as crypto::Secret) } + + /// Makes account unlocks expire and removes unused key files from memory + pub fn collect_garbage(&mut self) { + self.directory.collect_garbage(); + let utc = UTC::now(); + let expired_addresses = self.unlocks.read().unwrap().iter() + .filter(|&(_, unlock)| unlock.expires < utc) + .map(|(address, _)| address.clone()).collect::>(); + + for expired in expired_addresses { self.unlocks.write().unwrap().remove(&expired); } + } } fn derive_key_iterations(password: &str, salt: &H256, c: u32) -> (Bytes, Bytes) { @@ -362,14 +386,12 @@ impl EncryptedHashMap for SecretStore { } -#[cfg(test)] +#[cfg(all(test, feature="heavy-tests"))] mod vector_tests { use super::{derive_mac,derive_key_iterations}; use common::*; - #[test] - #[cfg(feature="heavy-tests")] fn mac_vector() { let password = "testpassword"; let salt = H256::from_str("ae3cd4e7013836a3df6bd7241b12db061dbe2c6785853cce422d148a624ce0bd").unwrap(); @@ -395,6 +417,7 @@ mod tests { use devtools::*; use common::*; use crypto::KeyPair; + use chrono::*; #[test] fn can_insert() { @@ -581,4 +604,32 @@ mod tests { let kp = KeyPair::from_secret(secret).unwrap(); assert_eq!(Address::from(kp.public().sha3()), addr); } + + #[test] + fn can_create_service() { + let temp = RandomTempPath::create_dir(); + let svc = AccountService::new_test(&temp); + assert!(svc.accounts().unwrap().is_empty()); + } + + #[test] + fn accounts_expire() { + use std::collections::hash_map::*; + + let temp = RandomTempPath::create_dir(); + let svc = AccountService::new_test(&temp); + let address = svc.new_account("pass").unwrap(); + svc.unlock_account(&address, "pass").unwrap(); + assert!(svc.account_secret(&address).is_ok()); + { + let ss_rw = svc.secret_store.write().unwrap(); + let mut ua_rw = ss_rw.unlocks.write().unwrap(); + let entry = ua_rw.entry(address); + if let Entry::Occupied(mut occupied) = entry { occupied.get_mut().expires = UTC::now() - Duration::minutes(1); } + } + + svc.tick(); + + assert!(svc.account_secret(&address).is_err()); + } } From 89dc6fa9ccc57233dccd23402dbd29eae2a60a8e Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Sun, 13 Mar 2016 14:46:45 +0100 Subject: [PATCH 2/4] io handlers --- parity/main.rs | 13 +++++++++++-- util/src/keys/store.rs | 1 + 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index b16801ad5..7a92e5570 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -445,6 +445,7 @@ impl Configuration { // Secret Store let account_service = Arc::new(AccountService::new()); + service.io().register_handler(account_service).expect("Error registering IO handler"); // Setup rpc if self.args.flag_jsonrpc || self.args.flag_rpc { @@ -468,6 +469,7 @@ impl Configuration { client: service.client(), info: Default::default(), sync: sync.clone(), + accounts: account_service.clone(), }); service.io().register_handler(io_handler).expect("Error registering IO handler"); @@ -559,20 +561,27 @@ impl Informant { const INFO_TIMER: TimerToken = 0; +const ACCOUNT_TICK_TIMER: TimerToken = 10; +const ACCOUNT_TICK_MS: u64 = 60000; + struct ClientIoHandler { client: Arc, sync: Arc, + accounts: Arc, info: Informant, } impl IoHandler for ClientIoHandler { fn initialize(&self, io: &IoContext) { io.register_timer(INFO_TIMER, 5000).expect("Error registering timer"); + io.register_timer(ACCOUNT_TICK_TIMER, ACCOUNT_TICK_MS).expect("Error registering account timer"); + } fn timeout(&self, _io: &IoContext, timer: TimerToken) { - if INFO_TIMER == timer { - self.info.tick(&self.client, &self.sync); + match timer { + INFO_TIMER => { self.info.tick(&self.client, &self.sync); } + ACCOUNT_TICK_TIMER => { self.accounts.tick(); } } } } diff --git a/util/src/keys/store.rs b/util/src/keys/store.rs index 610dda9f5..68121a82b 100644 --- a/util/src/keys/store.rs +++ b/util/src/keys/store.rs @@ -137,6 +137,7 @@ impl AccountService { } } + impl Default for SecretStore { fn default() -> Self { SecretStore::new() From a4f03100e9633d411d65a17fa568330c51be063f Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Sun, 13 Mar 2016 15:11:16 +0100 Subject: [PATCH 3/4] registering timer --- parity/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parity/main.rs b/parity/main.rs index 7a92e5570..1bfd2ced1 100644 --- a/parity/main.rs +++ b/parity/main.rs @@ -445,7 +445,6 @@ impl Configuration { // Secret Store let account_service = Arc::new(AccountService::new()); - service.io().register_handler(account_service).expect("Error registering IO handler"); // Setup rpc if self.args.flag_jsonrpc || self.args.flag_rpc { @@ -581,7 +580,8 @@ impl IoHandler for ClientIoHandler { fn timeout(&self, _io: &IoContext, timer: TimerToken) { match timer { INFO_TIMER => { self.info.tick(&self.client, &self.sync); } - ACCOUNT_TICK_TIMER => { self.accounts.tick(); } + ACCOUNT_TICK_TIMER => { self.accounts.tick(); }, + _ => {} } } } From c5edf237b2c17f56950016f3fcca1c77cfe416b3 Mon Sep 17 00:00:00 2001 From: Nikolay Volf Date: Sun, 13 Mar 2016 19:52:37 +0100 Subject: [PATCH 4/4] adding shrink-to-fit --- util/src/keys/directory.rs | 2 ++ util/src/keys/store.rs | 7 +++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/util/src/keys/directory.rs b/util/src/keys/directory.rs index d0d3393cd..a92bf4593 100644 --- a/util/src/keys/directory.rs +++ b/util/src/keys/directory.rs @@ -542,6 +542,8 @@ impl KeyDirectory { if removes.is_empty() { return; } let mut cache = self.cache.write().unwrap(); for key in removes { cache.remove(&key); } + + cache.shrink_to_fit(); } /// Reports how many keys are currently cached. diff --git a/util/src/keys/store.rs b/util/src/keys/store.rs index e0a505f79..78540bdb0 100644 --- a/util/src/keys/store.rs +++ b/util/src/keys/store.rs @@ -273,13 +273,16 @@ impl SecretStore { /// Makes account unlocks expire and removes unused key files from memory pub fn collect_garbage(&mut self) { + let mut garbage_lock = self.unlocks.write().unwrap(); self.directory.collect_garbage(); let utc = UTC::now(); - let expired_addresses = self.unlocks.read().unwrap().iter() + let expired_addresses = garbage_lock.iter() .filter(|&(_, unlock)| unlock.expires < utc) .map(|(address, _)| address.clone()).collect::>(); - for expired in expired_addresses { self.unlocks.write().unwrap().remove(&expired); } + for expired in expired_addresses { garbage_lock.remove(&expired); } + + garbage_lock.shrink_to_fit(); } }