From db165a2044aefd4ea736f26a3cb97aeba666cfc0 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 22 Jul 2020 10:11:49 +0200 Subject: [PATCH 1/5] Ensure we close old file handles hold by sysinfo --- client/service/src/metrics.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/service/src/metrics.rs b/client/service/src/metrics.rs index 8a483bc5a05ff..7d1afd049d763 100644 --- a/client/service/src/metrics.rs +++ b/client/service/src/metrics.rs @@ -206,6 +206,7 @@ impl MetricsService { fn process_info(&mut self) -> ProcessInfo { let pid = self.pid.clone().expect("unix always has a pid. qed"); + self.system.clear_procs(); // ensure we close old, released tasks handles let mut info = self.process_info_for(&pid); let process = procfs::process::Process::new(pid).expect("Our process exists. qed."); info.threads = process.stat().ok().map(|s| @@ -240,6 +241,7 @@ impl MetricsService { } fn process_info(&mut self) -> ProcessInfo { + self.system.clear_procs(); // ensure we close old, released tasks handles self.pid.map(|pid| self.process_info_for(&pid)).unwrap_or_default() } } From 1c1a0fe443bb0069e411156e40887e573940d775 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 22 Jul 2020 11:27:34 +0200 Subject: [PATCH 2/5] Dropping is needed unfortunately --- client/service/src/metrics.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/client/service/src/metrics.rs b/client/service/src/metrics.rs index 7d1afd049d763..4132c132c9251 100644 --- a/client/service/src/metrics.rs +++ b/client/service/src/metrics.rs @@ -206,7 +206,6 @@ impl MetricsService { fn process_info(&mut self) -> ProcessInfo { let pid = self.pid.clone().expect("unix always has a pid. qed"); - self.system.clear_procs(); // ensure we close old, released tasks handles let mut info = self.process_info_for(&pid); let process = procfs::process::Process::new(pid).expect("Our process exists. qed."); info.threads = process.stat().ok().map(|s| @@ -241,7 +240,6 @@ impl MetricsService { } fn process_info(&mut self) -> ProcessInfo { - self.system.clear_procs(); // ensure we close old, released tasks handles self.pid.map(|pid| self.process_info_for(&pid)).unwrap_or_default() } } @@ -284,6 +282,10 @@ impl MetricsService { #[cfg(all(any(unix, windows), not(target_os = "android"), not(target_os = "ios")))] fn process_info_for(&mut self, pid: &sysinfo::Pid) -> ProcessInfo { + // FIXME: sysinfo::System leaks fd-handlers on Linux. We have to drop it to clean this up + // https://github.com/GuillaumeGomez/sysinfo/issues/352 + self.system = sysinfo::System::new(); + let mut info = ProcessInfo::default(); if self.system.refresh_process(*pid) { let prc = self.system.get_process(*pid) From 004e7865d4400e46c902bfdfe251e9abd9ebe2fd Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 22 Jul 2020 12:35:12 +0200 Subject: [PATCH 3/5] enable process refreshing, ignore result from refresh_process --- client/service/src/metrics.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/client/service/src/metrics.rs b/client/service/src/metrics.rs index 4132c132c9251..d8d6f25e778da 100644 --- a/client/service/src/metrics.rs +++ b/client/service/src/metrics.rs @@ -284,15 +284,14 @@ impl MetricsService { fn process_info_for(&mut self, pid: &sysinfo::Pid) -> ProcessInfo { // FIXME: sysinfo::System leaks fd-handlers on Linux. We have to drop it to clean this up // https://github.com/GuillaumeGomez/sysinfo/issues/352 - self.system = sysinfo::System::new(); + self.system = sysinfo::System::new_with_specifics(sysinfo::RefreshKind::new().with_processes()); let mut info = ProcessInfo::default(); - if self.system.refresh_process(*pid) { - let prc = self.system.get_process(*pid) - .expect("Above refresh_process succeeds, this must be Some(), qed"); + self.system.refresh_process(*pid); + self.system.get_process(*pid).map(|prc| { info.cpu_usage = prc.cpu_usage().into(); info.memory = prc.memory(); - } + }); info } From 26c04635b4c8e2691dc444a64f302d3a7997eb07 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 22 Jul 2020 12:47:17 +0200 Subject: [PATCH 4/5] jumping to proposed patch --- Cargo.lock | 5 ++--- client/service/Cargo.toml | 2 +- client/service/src/metrics.rs | 8 ++------ 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a79f6134a88d4..7bd0cebb98dde 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8815,9 +8815,8 @@ dependencies = [ [[package]] name = "sysinfo" -version = "0.14.13" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eec476c3d107e7fc2c445e4edc26836c49ba5be0dae74146ee94ecb62759c31d" +version = "0.14.14" +source = "git+https://github.com/GuillaumeGomez/sysinfo?branch=tasks-cleanup#9d7ef472900b95230c5ad33b96ac3bc936860b6b" dependencies = [ "cfg-if", "doc-comment", diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 7d321d535fa54..0c7955a6007f3 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -39,7 +39,7 @@ pin-project = "0.4.8" hash-db = "0.15.2" serde = "1.0.101" serde_json = "1.0.41" -sysinfo = "0.14.3" +sysinfo = { git = "https://github.com/GuillaumeGomez/sysinfo", branch = "tasks-cleanup"} sc-keystore = { version = "2.0.0-rc5", path = "../keystore" } sp-io = { version = "2.0.0-rc5", path = "../../primitives/io" } sp-runtime = { version = "2.0.0-rc5", path = "../../primitives/runtime" } diff --git a/client/service/src/metrics.rs b/client/service/src/metrics.rs index d8d6f25e778da..bac8b38d42390 100644 --- a/client/service/src/metrics.rs +++ b/client/service/src/metrics.rs @@ -199,7 +199,7 @@ impl MetricsService { Self { metrics, - system: sysinfo::System::new(), + system: sysinfo::System::new_with_specifics(sysinfo::RefreshKind::new().with_processes()), pid: Some(process.pid), } } @@ -234,7 +234,7 @@ impl MetricsService { fn inner_new(metrics: Option) -> Self { Self { metrics, - system: sysinfo::System::new(), + system: sysinfo::System::new_with_specifics(sysinfo::RefreshKind::new().with_processes()), pid: sysinfo::get_current_pid().ok(), } } @@ -282,10 +282,6 @@ impl MetricsService { #[cfg(all(any(unix, windows), not(target_os = "android"), not(target_os = "ios")))] fn process_info_for(&mut self, pid: &sysinfo::Pid) -> ProcessInfo { - // FIXME: sysinfo::System leaks fd-handlers on Linux. We have to drop it to clean this up - // https://github.com/GuillaumeGomez/sysinfo/issues/352 - self.system = sysinfo::System::new_with_specifics(sysinfo::RefreshKind::new().with_processes()); - let mut info = ProcessInfo::default(); self.system.refresh_process(*pid); self.system.get_process(*pid).map(|prc| { From f6ae6332a258e047488d20ebb880fbac0bfe6739 Mon Sep 17 00:00:00 2001 From: Benjamin Kampmann Date: Wed, 22 Jul 2020 15:57:25 +0200 Subject: [PATCH 5/5] switch to latest sysinfo --- Cargo.lock | 5 +++-- client/service/Cargo.toml | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7bd0cebb98dde..af5ad77684b30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8815,8 +8815,9 @@ dependencies = [ [[package]] name = "sysinfo" -version = "0.14.14" -source = "git+https://github.com/GuillaumeGomez/sysinfo?branch=tasks-cleanup#9d7ef472900b95230c5ad33b96ac3bc936860b6b" +version = "0.14.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2983daff11a197c7c406b130579bc362177aa54cf2cc1f34d6ac88fccaa6a5e1" dependencies = [ "cfg-if", "doc-comment", diff --git a/client/service/Cargo.toml b/client/service/Cargo.toml index 0c7955a6007f3..3511c29038299 100644 --- a/client/service/Cargo.toml +++ b/client/service/Cargo.toml @@ -39,7 +39,7 @@ pin-project = "0.4.8" hash-db = "0.15.2" serde = "1.0.101" serde_json = "1.0.41" -sysinfo = { git = "https://github.com/GuillaumeGomez/sysinfo", branch = "tasks-cleanup"} +sysinfo = "0.14.15" sc-keystore = { version = "2.0.0-rc5", path = "../keystore" } sp-io = { version = "2.0.0-rc5", path = "../../primitives/io" } sp-runtime = { version = "2.0.0-rc5", path = "../../primitives/runtime" }