Remove rental crate and implement the required functionality

docs/CODEOWNERS

@@ -66,3 +66,8 @@
 
 # Prometheus endpoint
 /utils/prometheus/ @mxinden
+
+# Tracing
+/client/tracing/ @mattrutherford
+/primitives/tracing/src/proxy.rs @mattrutherford
+/primitives/tracing/src/proxied_span.rs @mattrutherford

primitives/tracing/Cargo.toml

@@ -13,9 +13,8 @@ targets = ["x86_64-unknown-linux-gnu"]
 
 [dependencies]
 tracing = { version = "0.1.13", optional = true }
-rental = { version = "0.5.5", optional = true }
 log = { version = "0.4.8", optional = true }
 
 [features]
 default = [ "std" ]
-std = [ "tracing", "rental", "log" ]
+std = [ "tracing", "log" ]

primitives/tracing/src/lib.rs

@@ -30,17 +30,16 @@
 //! the associated Fields mentioned above.
 #![cfg_attr(not(feature = "std"), no_std)]
 
-#[cfg(feature = "std")]
-#[macro_use]
-extern crate rental;
-
 #[cfg(feature = "std")]
 #[doc(hidden)]
 pub use tracing;
 
 #[cfg(feature = "std")]
 pub mod proxy;
 
+#[cfg(feature = "std")]
+mod proxied_span;
+
 #[cfg(feature = "std")]
 use std::sync::atomic::{AtomicBool, Ordering};
 

primitives/tracing/src/proxied_span.rs

@@ -0,0 +1,68 @@
+// Copyright 2020 Parity Technologies (UK) Ltd.
+// This file is part of Substrate.
+
+// Substrate is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Substrate is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+
+//! Provides the struct `ProxiedSpan`, which encapsulates the necessary
+//! associated unsafe code and provides a safe interface to the proxy.
+
+use std::mem::{transmute, ManuallyDrop};
+use std::pin::Pin;
+use tracing::{span::Entered, Span};
+
+/// Container for a proxied tracing Span and it's associated guard
+pub struct ProxiedSpan {
+	id: u64,
+	// Crucial that `guard` must be dropped first - ensured by `drop` impl.
+	guard: ManuallyDrop<Entered<'static>>,
+	span: ManuallyDrop<Pin<Box<Span>>>,
+}
+
+impl ProxiedSpan {
+	/// Enter the supplied span and return a new instance of ProxiedTrace containing it
+	pub fn enter_span(id: u64, span: Pin<Box<Span>>) -> Self {
+		let span = ManuallyDrop::new(span);
+		// Transmute to static lifetime to enable guard to be stored
+		// along with the span that it references
+		let guard = unsafe {
+			ManuallyDrop::new(transmute::<Entered<'_>, Entered<'static>>(span.enter()))
+		};
+
+		ProxiedSpan {
+			id,
+			guard,
+			span,
+		}
+	}
+
+	/// Return copy of the span id
+	pub fn id(&self) -> u64 {
+		self.id
+	}
+
+	/// Return immutable reference to the span
+	pub fn span(&self) -> &Span {
+		&*self.span
+	}
+}
+
+impl Drop for ProxiedSpan {
+	fn drop(&mut self) {
+		unsafe {
+			// Ensure guard is dropped before span to prevent invalid reference
+			ManuallyDrop::drop(&mut self.guard);
+			ManuallyDrop::drop(&mut self.span);
+		}
+	}
+}

primitives/tracing/src/proxy.rs

@@ -17,8 +17,8 @@
 //! Proxy to allow entering tracing spans from wasm.
 //!
 //! Use `enter_span` and `exit_span` to surround the code that you wish to trace
-use rental;
 use tracing::info_span;
+use crate::proxied_span::ProxiedSpan;
 
 /// Used to identify a proxied WASM trace
 pub const WASM_TRACE_IDENTIFIER: &'static str = "WASM_TRACE";
@@ -29,21 +29,11 @@ pub const WASM_NAME_KEY: &'static str = "proxied_wasm_name";
 
 const MAX_SPANS_LEN: usize = 1000;
 
-rental! {
-	pub mod rent_span {
-		#[rental]
-		pub struct SpanAndGuard {
-			span: Box<tracing::Span>,
-			guard: tracing::span::Entered<'span>,
-		}
-	}
-}
-
 /// Requires a tracing::Subscriber to process span traces,
 /// this is available when running with client (and relevant cli params).
 pub struct TracingProxy {
 	next_id: u64,
-	spans: Vec<(u64, rent_span::SpanAndGuard)>,
+	spans: Vec<ProxiedSpan>,
 }
 
 impl Drop for TracingProxy {
@@ -53,8 +43,8 @@ impl Drop for TracingProxy {
 				target: "tracing",
 				"Dropping TracingProxy with {} un-exited spans, marking as not valid", self.spans.len()
 			);
-			while let Some((_, mut sg)) = self.spans.pop() {
-				sg.rent_all_mut(|s| { s.span.record("is_valid_trace", &false); });
+			while let Some(proxied_span) = self.spans.pop() {
+				proxied_span.span().record("is_valid_trace", &false);
 			}
 		}
 	}
@@ -75,13 +65,17 @@ impl TracingProxy {
 	pub fn enter_span(&mut self, proxied_wasm_target: &str, proxied_wasm_name: &str) -> u64 {
 		// The identifiers `proxied_wasm_target` and `proxied_wasm_name` must match their associated const,
 		// WASM_TARGET_KEY and WASM_NAME_KEY.
-		let span = info_span!(WASM_TRACE_IDENTIFIER, is_valid_trace = true, proxied_wasm_target, proxied_wasm_name);
-		self.next_id += 1;
-		let sg = rent_span::SpanAndGuard::new(
-			Box::new(span),
-			|span| span.enter(),
+		let span = Box::pin(
+			info_span!(
+				WASM_TRACE_IDENTIFIER,
+				is_valid_trace = true,
+				proxied_wasm_target,
+				proxied_wasm_name
+			)
 		);
-		self.spans.push((self.next_id, sg));
+		self.next_id += 1;
+		let proxied_span = ProxiedSpan::enter_span(self.next_id, span);
+		self.spans.push(proxied_span);
 		if self.spans.len() > MAX_SPANS_LEN {
 			// This is to prevent unbounded growth of Vec and could mean one of the following:
 			// 1. Too many nested spans, or MAX_SPANS_LEN is too low.
@@ -90,29 +84,29 @@ impl TracingProxy {
 				target: "tracing",
 				"TracingProxy MAX_SPANS_LEN exceeded, removing oldest span."
 			);
-			let mut sg = self.spans.remove(0).1;
-			sg.rent_all_mut(|s| { s.span.record("is_valid_trace", &false); });
+			let proxied_span = self.spans.remove(0);
+			proxied_span.span().record("is_valid_trace", &false);
 		}
 		self.next_id
 	}
 
 	/// Exit a span by dropping it along with it's associated guard.
 	pub fn exit_span(&mut self, id: u64) {
-		if self.spans.last().map(|l| id > l.0).unwrap_or(true) {
+		if self.spans.last().map(|proxied_span| id > proxied_span.id()).unwrap_or(true) {
 			log::warn!(target: "tracing", "Span id not found in TracingProxy: {}", id);
 			return;
 		}
-		let mut last_span = self.spans.pop().expect("Just checked that there is an element to pop; qed");
-		while id < last_span.0 {
+		let mut proxied_span = self.spans.pop().expect("Just checked that there is an element to pop; qed");
+		while id < proxied_span.id() {
 			log::warn!(
 				target: "tracing",
 				"TracingProxy Span ids not equal! id parameter given: {}, last span: {}",
 				id,
-				last_span.0,
+				proxied_span.id(),
 			);
-			last_span.1.rent_all_mut(|s| { s.span.record("is_valid_trace", &false); });
-			if let Some(s) = self.spans.pop() {
-				last_span = s;
+			proxied_span.span().record("is_valid_trace", &false);
+			if let Some(ps) = self.spans.pop() {
+				proxied_span = ps;
 			} else {
 				log::warn!(target: "tracing", "Span id not found in TracingProxy {}", id);
 				return;
@@ -140,7 +134,7 @@ mod tests {
 		let _spans = create_spans(&mut proxy, MAX_SPANS_LEN + 10);
 		assert_eq!(proxy.spans.len(), MAX_SPANS_LEN);
 		// ensure oldest spans removed
-		assert_eq!(proxy.spans[0].0, 11);
+		assert_eq!(proxy.spans[0].id(), 11);
 	}
 
 	#[test]