From 68713cbcf19a91c2028f9e5011bf5d8c6a4e0074 Mon Sep 17 00:00:00 2001 From: Pauan Date: Thu, 15 Feb 2018 08:50:42 -1000 Subject: [PATCH 01/13] Adding in DoneHandle to clean up memory leaks --- src/lib.rs | 2 +- src/webcore/promise.rs | 56 +++++++++++++++++++++++++++-------- src/webcore/promise_future.rs | 3 +- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 046bd04d..2089e01e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -174,7 +174,7 @@ pub use webcore::reference_type::ReferenceType; pub use webcore::serialization::JsSerialize; #[cfg(feature = "experimental_features_which_may_break_on_minor_version_bumps")] -pub use webcore::promise::Promise; +pub use webcore::promise::{Promise, DoneHandle}; #[cfg(all( feature = "futures", diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index 89b33705..f968b6aa 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -16,6 +16,22 @@ use futures::future::Future; use super::promise_future::PromiseFuture; +#[derive( Debug, Clone )] +pub struct DoneHandle { + callback: Value, + done: Value, +} + +impl Drop for DoneHandle { + fn drop( &mut self ) { + js! { @(no_return) + @{&self.done}[0] = true; + @{&self.callback}.drop(); + } + } +} + + /// A `Promise` object represents the eventual completion (or failure) of an asynchronous operation, and its resulting value. /// /// In most situations you shouldn't use this, use [`PromiseFuture`](struct.PromiseFuture.html) instead. @@ -154,7 +170,7 @@ impl Promise { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then) // https://www.ecma-international.org/ecma-262/6.0/#sec-performpromisethen - pub fn done< A, B, F >( &self, callback: F ) + pub fn done< A, B, F >( &self, callback: F ) -> DoneHandle where A: TryFrom< Value >, B: TryFrom< Value >, // TODO these Debug constraints are only needed because of unwrap @@ -167,6 +183,7 @@ impl Promise { // TODO figure out a way to avoid the unwrap let value: A = value.try_into().unwrap(); Ok( value ) + } else { // TODO figure out a way to avoid the unwrap let value: B = value.try_into().unwrap(); @@ -176,15 +193,29 @@ impl Promise { callback( value ); }; - js! { @(no_return) - var callback = @{Once( callback )}; + let callback = js!( return @{Once( callback )}; ); + + let done = js!( + var callback = @{&callback}; + var done = [ false ]; // TODO don't swallow any errors thrown inside callback @{self}.then( function ( value ) { - callback( value, true ); + if ( !done[0] ) { + callback( value, true ); + } }, function ( value ) { - callback( value, false ); + if ( !done[0] ) { + callback( value, false ); + } } ); + + return done; + ); + + DoneHandle { + callback, + done } } @@ -209,16 +240,15 @@ impl Promise { let ( sender, receiver ) = channel(); - self.done( |value| { - // TODO is this correct ? - match sender.send( value ) { - Ok( _ ) => {}, - Err( _ ) => {}, - }; - } ); - PromiseFuture { future: receiver, + _done_handle: self.done( |value| { + // TODO is this correct ? + match sender.send( value ) { + Ok( _ ) => {}, + Err( _ ) => {}, + }; + } ) } } } diff --git a/src/webcore/promise_future.rs b/src/webcore/promise_future.rs index e4ddb5b2..44ca514d 100644 --- a/src/webcore/promise_future.rs +++ b/src/webcore/promise_future.rs @@ -5,7 +5,7 @@ use webapi::error; use futures::{Future, Poll, Async}; use futures::unsync::oneshot::Receiver; use webcore::promise_executor::spawn; -use super::promise::Promise; +use super::promise::{Promise, DoneHandle}; /// This allows you to use a JavaScript [`Promise`](struct.Promise.html) as if it is a Rust [`Future`](https://docs.rs/futures/0.1.*/futures/future/trait.Future.html). @@ -21,6 +21,7 @@ use super::promise::Promise; /// ``` pub struct PromiseFuture< Value, Error = error::Error > { pub(crate) future: Receiver< Result< Value, Error > >, + pub(crate) _done_handle: DoneHandle, } impl PromiseFuture< (), () > { From eaccc18288bad389ee55e326416b4df631f8d620 Mon Sep 17 00:00:00 2001 From: Pauan Date: Thu, 15 Feb 2018 09:19:51 -1000 Subject: [PATCH 02/13] Adding in more examples for DoneHandle --- src/webcore/promise.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index f968b6aa..b27b0054 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -16,6 +16,7 @@ use futures::future::Future; use super::promise_future::PromiseFuture; +/// #[derive( Debug, Clone )] pub struct DoneHandle { callback: Value, @@ -178,7 +179,7 @@ impl Promise { B::Error: std::fmt::Debug, F: FnOnce( Result< A, B > ) + 'static { - let callback = |value: Value, success: bool| { + let callback = move |value: Value, success: bool| { let value: Result< A, B > = if success { // TODO figure out a way to avoid the unwrap let value: A = value.try_into().unwrap(); @@ -215,7 +216,7 @@ impl Promise { DoneHandle { callback, - done + done, } } @@ -248,7 +249,7 @@ impl Promise { Ok( _ ) => {}, Err( _ ) => {}, }; - } ) + } ), } } } From 9209bf53c16b358adaddd82c19b717324fa97306 Mon Sep 17 00:00:00 2001 From: Pauan Date: Thu, 15 Feb 2018 09:34:52 -1000 Subject: [PATCH 03/13] Adding in documentation --- src/webcore/promise.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index b27b0054..33a150f8 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -16,7 +16,9 @@ use futures::future::Future; use super::promise_future::PromiseFuture; +/// This is used to cleanup the [`done`](struct.Promise.html#method.done) callback. /// +/// See the documentation for [`done`](struct.Promise.html#method.done) for more information. #[derive( Debug, Clone )] pub struct DoneHandle { callback: Value, @@ -156,12 +158,22 @@ impl Promise { /// /// The `callback` is guaranteed to be called asynchronously even if the `Promise` is already succeeded / failed. /// - /// If the `Promise` never succeeds / fails then the `callback` will never be called, and it will leak memory. + /// If the `Promise` never succeeds / fails then the `callback` will never be called. + /// + /// This method returns a [`DoneHandle`](struct.DoneHandle.html). When the [`DoneHandle`](struct.DoneHandle.html) + /// is dropped it will drop the `callback` and the `callback` will never be called. This is useful if you are + /// no longer interested in the `Promise`'s result. + /// + /// But if you *are* interested in the `Promise`'s result, then you need to make sure to keep the handle alive + /// until after the callback is called. + /// + /// Dropping the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises + /// do not support cancellation. /// /// # Examples /// /// ```rust - /// promise.done(|result| { + /// let handle = promise.done(|result| { /// match result { /// Ok(success) => { ... }, /// Err(error) => { ... }, From 08d26b9b18bf4a94238f5cffbcbd26ec8a5dda21 Mon Sep 17 00:00:00 2001 From: Pauan Date: Thu, 22 Feb 2018 15:08:52 -1000 Subject: [PATCH 04/13] Adding in Cancel and AutoCancel --- src/lib.rs | 2 +- src/webapi/event_target.rs | 28 ++++++++-------- src/webapi/mutation_observer.rs | 15 +++++---- src/webapi/window.rs | 15 +++++---- src/webcore/cancel.rs | 59 +++++++++++++++++++++++++++++++++ src/webcore/mod.rs | 1 + src/webcore/promise.rs | 19 ++++++----- src/webcore/promise_future.rs | 3 +- 8 files changed, 103 insertions(+), 39 deletions(-) create mode 100644 src/webcore/cancel.rs diff --git a/src/lib.rs b/src/lib.rs index 2089e01e..28fbf8ef 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -207,7 +207,7 @@ pub mod web { }; pub use webapi::cross_origin_setting::CrossOriginSetting; pub use webapi::date::Date; - pub use webapi::event_target::{IEventTarget, EventTarget, EventListenerHandle}; + pub use webapi::event_target::{IEventTarget, EventTarget, EventListener}; pub use webapi::window::RequestAnimationFrameHandle; pub use webapi::node::{INode, Node, CloneKind}; pub use webapi::element::{IElement, Element}; diff --git a/src/webapi/event_target.rs b/src/webapi/event_target.rs index 3dd20aa1..5d2da66c 100644 --- a/src/webapi/event_target.rs +++ b/src/webapi/event_target.rs @@ -4,34 +4,33 @@ use webcore::value::Reference; use webcore::try_from::TryInto; use webcore::reference_type::ReferenceType; use webapi::event::{ConcreteEvent, IEvent}; +use webcore::cancel::{Cancel, AutoCancel}; use private::TODO; /// A handle to a particular event listener. -pub struct EventListenerHandle { +pub struct EventListener { event_type: &'static str, reference: Reference, listener_reference: Reference } -impl fmt::Debug for EventListenerHandle { +impl fmt::Debug for EventListener { fn fmt( &self, formatter: &mut fmt::Formatter ) -> fmt::Result { - write!( formatter, "EventListenerHandle {{ event_type: {}, reference: {:?} }}", self.event_type, self.reference ) + write!( formatter, "EventListener {{ event_type: {}, reference: {:?} }}", self.event_type, self.reference ) } } -impl EventListenerHandle { - /// Removes the handler from the [IEventTarget](trait.IEventTarget.html) on +impl Cancel for EventListener { + /// Removes the listener from the [IEventTarget](trait.IEventTarget.html) on /// which it was previously registered. /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener) // https://dom.spec.whatwg.org/#ref-for-dom-eventtarget-removeeventlistener%E2%91%A0 - pub fn remove( self ) { + fn cancel( &mut self ) { js! { @(no_return) - var self = @{self.reference}; - var event_type = @{self.event_type}; - var listener = @{self.listener_reference}; + var listener = @{&self.listener_reference}; + @{&self.reference}.removeEventListener( @{self.event_type}, listener ); listener.drop(); - self.removeEventListener( event_type, listener ); } } } @@ -42,26 +41,27 @@ impl EventListenerHandle { /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget) // https://dom.spec.whatwg.org/#eventtarget pub trait IEventTarget: ReferenceType { - /// Adds given event handler to the list the list of event listeners for + /// Adds given event handler to the list of event listeners for /// the specified `EventTarget` on which it's called. /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener) // https://dom.spec.whatwg.org/#ref-for-dom-eventtarget-addeventlistener%E2%91%A0 - fn add_event_listener< T, F >( &self, listener: F ) -> EventListenerHandle + fn add_event_listener< T, F >( &self, listener: F ) -> AutoCancel< EventListener > where T: ConcreteEvent, F: FnMut( T ) + 'static { let reference = self.as_ref(); + let listener_reference = js! { var listener = @{listener}; @{reference}.addEventListener( @{T::EVENT_TYPE}, listener ); return listener; }.try_into().unwrap(); - EventListenerHandle { + AutoCancel::new( EventListener { event_type: T::EVENT_TYPE, reference: reference.clone(), listener_reference: listener_reference - } + } ) } /// Dispatches an `Event` at this `EventTarget`, invoking the affected event listeners in the diff --git a/src/webapi/mutation_observer.rs b/src/webapi/mutation_observer.rs index 71af3335..002bf44b 100644 --- a/src/webapi/mutation_observer.rs +++ b/src/webapi/mutation_observer.rs @@ -3,6 +3,7 @@ use webcore::value::{Reference, Value, ConversionError}; use webapi::node_list::NodeList; use webcore::try_from::{TryFrom, TryInto}; use webapi::node::{INode, Node}; +use webcore::cancel::{Cancel, AutoCancel}; use private::TODO; /// Provides a way to receive notifications about changes to the DOM. @@ -62,17 +63,17 @@ impl MutationObserver { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver#Constructor) // https://dom.spec.whatwg.org/#ref-for-dom-mutationobserver-mutationobserver - pub fn new< F >( callback: F ) -> MutationObserverHandle + pub fn new< F >( callback: F ) -> AutoCancel< MutationObserverHandle > where F: FnMut( Vec< MutationRecord >, Self ) + 'static { let callback_reference: Reference = js! ( return @{callback}; ).try_into().unwrap(); - MutationObserverHandle { + AutoCancel::new( MutationObserverHandle { callback_reference: callback_reference.clone(), mutation_observer: js! ( return new MutationObserver( @{callback_reference} ); ).try_into().unwrap(), - } + } ) } /// Starts observing changes to the `target`. @@ -159,7 +160,7 @@ impl MutationObserver { /// This is created by the [`MutationObserver::new`](struct.MutationObserver.html#method.new) method, and /// it can use the same methods as [`MutationObserver`](struct.MutationObserver.html). /// -/// When the `MutationObserverHandle` is dropped, the [`disconnect`](#method.disconnect) +/// When the `MutationObserverHandle` is cancelled, the [`disconnect`](#method.disconnect) /// method will automatically be called. #[ derive( Debug ) ] pub struct MutationObserverHandle { @@ -171,14 +172,14 @@ impl std::ops::Deref for MutationObserverHandle { type Target = MutationObserver; #[inline] - fn deref(&self) -> &Self::Target { + fn deref( &self ) -> &Self::Target { &self.mutation_observer } } -impl Drop for MutationObserverHandle { +impl Cancel for MutationObserverHandle { #[inline] - fn drop( &mut self ) { + fn cancel( &mut self ) { self.disconnect(); js! { @(no_return) diff --git a/src/webapi/window.rs b/src/webapi/window.rs index 1aa9fef5..a2facf5a 100644 --- a/src/webapi/window.rs +++ b/src/webapi/window.rs @@ -7,21 +7,22 @@ use webapi::location::Location; use webapi::history::History; use webcore::once::Once; use webcore::value::Value; +use webcore::cancel::{Cancel, AutoCancel}; /// A handle to a pending animation frame request. #[derive(Debug)] pub struct RequestAnimationFrameHandle(Value); -impl RequestAnimationFrameHandle { +impl Cancel for RequestAnimationFrameHandle { /// Cancels an animation frame request. /// /// [(Javascript docs)](https://developer.mozilla.org/en-US/docs/Web/API/Window/cancelAnimationFrame) - pub fn cancel(self) { - js!{ - var val = @{self.0}; + fn cancel( &mut self ) { + js! { @(no_return) + var val = @{&self.0}; val.window.cancelAnimationFrame(val.request); val.callback.drop(); - }; + } } } @@ -123,13 +124,13 @@ impl Window { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame) // https://html.spec.whatwg.org/#the-window-object:dom-window-requestanimationframe - pub fn request_animation_frame< F: FnOnce(f64) + 'static>( &self, callback: F) -> RequestAnimationFrameHandle { + pub fn request_animation_frame< F: FnOnce(f64) + 'static>( &self, callback: F) -> AutoCancel< RequestAnimationFrameHandle > { let values: Value = js!{ var callback = @{Once(callback)}; var request = @{self}.requestAnimationFrame(callback); return { request: request, callback: callback, window: @{self} }; }; - RequestAnimationFrameHandle(values) + AutoCancel::new( RequestAnimationFrameHandle(values) ) } /// Returns the global [History](struct.History.html) object, which provides methods to diff --git a/src/webcore/cancel.rs b/src/webcore/cancel.rs new file mode 100644 index 00000000..691562ac --- /dev/null +++ b/src/webcore/cancel.rs @@ -0,0 +1,59 @@ +use std::ops::{Deref, DerefMut}; + + +pub trait Cancel { + fn cancel( &mut self ); +} + + +#[must_use = " + The AutoCancel will be automatically cancelled when it is dropped. + You probably don't want this to happen. + How to fix this: either use the AutoCancel, or use .leak() which will cause it to not be cancelled (this will leak memory!) +"] +#[derive(Debug)] +pub struct AutoCancel< A: Cancel >( Option< A > ); + +impl< A: Cancel > AutoCancel< A > { + #[inline] + pub fn new( canceler: A ) -> Self { + AutoCancel( Some( canceler ) ) + } + + #[inline] + pub fn leak( mut self ) -> A { + self.0.take().unwrap() + } +} + +impl< A: Cancel > Drop for AutoCancel< A > { + #[inline] + fn drop( &mut self ) { + match self.0 { + Some( ref mut canceler ) => canceler.cancel(), + None => {}, + } + } +} + +impl< A: Cancel > Deref for AutoCancel< A > { + type Target = A; + + #[inline] + fn deref( &self ) -> &Self::Target { + match self.0 { + Some( ref canceler ) => canceler, + None => unreachable!(), + } + } +} + +impl< A: Cancel > DerefMut for AutoCancel< A > { + #[inline] + fn deref_mut( &mut self ) -> &mut Self::Target { + match self.0 { + Some( ref mut canceler ) => canceler, + None => unreachable!(), + } + } +} diff --git a/src/webcore/mod.rs b/src/webcore/mod.rs index 7088fb94..8b26b51e 100644 --- a/src/webcore/mod.rs +++ b/src/webcore/mod.rs @@ -17,6 +17,7 @@ pub mod once; pub mod instance_of; pub mod reference_type; pub mod promise; +pub mod cancel; #[cfg(feature = "futures")] pub mod promise_future; diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index 33a150f8..58953c4e 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -2,6 +2,7 @@ use std; use webcore::once::Once; use webcore::value::{Value, Reference}; use webcore::try_from::{TryInto, TryFrom}; +use webcore::cancel::{Cancel, AutoCancel}; #[cfg(feature = "futures")] use webcore::serialization::JsSerialize; @@ -25,8 +26,8 @@ pub struct DoneHandle { done: Value, } -impl Drop for DoneHandle { - fn drop( &mut self ) { +impl Cancel for DoneHandle { + fn cancel( &mut self ) { js! { @(no_return) @{&self.done}[0] = true; @{&self.callback}.drop(); @@ -161,13 +162,13 @@ impl Promise { /// If the `Promise` never succeeds / fails then the `callback` will never be called. /// /// This method returns a [`DoneHandle`](struct.DoneHandle.html). When the [`DoneHandle`](struct.DoneHandle.html) - /// is dropped it will drop the `callback` and the `callback` will never be called. This is useful if you are + /// is cancelled it will drop the `callback` and the `callback` will never be called. This is useful if you are /// no longer interested in the `Promise`'s result. /// - /// But if you *are* interested in the `Promise`'s result, then you need to make sure to keep the handle alive - /// until after the callback is called. + /// But if you *are* interested in the `Promise`'s result, then you either need to make sure to keep the handle + /// alive until after the callback is called, or you need to use the `leak` method. /// - /// Dropping the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises + /// Cancelling the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises /// do not support cancellation. /// /// # Examples @@ -183,7 +184,7 @@ impl Promise { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then) // https://www.ecma-international.org/ecma-262/6.0/#sec-performpromisethen - pub fn done< A, B, F >( &self, callback: F ) -> DoneHandle + pub fn done< A, B, F >( &self, callback: F ) -> AutoCancel< DoneHandle > where A: TryFrom< Value >, B: TryFrom< Value >, // TODO these Debug constraints are only needed because of unwrap @@ -226,10 +227,10 @@ impl Promise { return done; ); - DoneHandle { + AutoCancel::new( DoneHandle { callback, done, - } + } ) } /// This method should rarely be needed, instead use [`value.try_into()`](unstable/trait.TryInto.html) to convert directly from a [`Value`](enum.Value.html) into a [`PromiseFuture`](struct.PromiseFuture.html). diff --git a/src/webcore/promise_future.rs b/src/webcore/promise_future.rs index 44ca514d..78a333f8 100644 --- a/src/webcore/promise_future.rs +++ b/src/webcore/promise_future.rs @@ -5,6 +5,7 @@ use webapi::error; use futures::{Future, Poll, Async}; use futures::unsync::oneshot::Receiver; use webcore::promise_executor::spawn; +use webcore::cancel::AutoCancel; use super::promise::{Promise, DoneHandle}; @@ -21,7 +22,7 @@ use super::promise::{Promise, DoneHandle}; /// ``` pub struct PromiseFuture< Value, Error = error::Error > { pub(crate) future: Receiver< Result< Value, Error > >, - pub(crate) _done_handle: DoneHandle, + pub(crate) _done_handle: AutoCancel< DoneHandle >, } impl PromiseFuture< (), () > { From ac9ebe0623af42208636801621b55a7a82e6d513 Mon Sep 17 00:00:00 2001 From: Pauan Date: Thu, 22 Feb 2018 15:31:07 -1000 Subject: [PATCH 05/13] Improving the must_use warning message --- src/webcore/cancel.rs | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/webcore/cancel.rs b/src/webcore/cancel.rs index 691562ac..a552998c 100644 --- a/src/webcore/cancel.rs +++ b/src/webcore/cancel.rs @@ -7,9 +7,15 @@ pub trait Cancel { #[must_use = " - The AutoCancel will be automatically cancelled when it is dropped. - You probably don't want this to happen. - How to fix this: either use the AutoCancel, or use .leak() which will cause it to not be cancelled (this will leak memory!) + + The AutoCancel is unused, which causes it to be immediately cancelled. + You probably don't want that to happen. + + How to fix this: + 1) Store the AutoCancel in a variable or data structure + 2) Use .leak() which will cause it to not be cancelled (this *will* leak memory!) + + See the documentation for more details. "] #[derive(Debug)] pub struct AutoCancel< A: Cancel >( Option< A > ); @@ -57,3 +63,28 @@ impl< A: Cancel > DerefMut for AutoCancel< A > { } } } + + +#[cfg(test)] +mod tests { + use super::{Cancel, AutoCancel}; + + struct Foo( bool ); + + impl Foo { + fn new() -> AutoCancel< Foo > { + AutoCancel::new( Foo( false ) ) + } + } + + impl Cancel for Foo { + fn cancel( &mut self ) { + self.0 = true; + } + } + + #[test] + fn unused() { + Foo::new(); + } +} From 975d13e4855cda1f2dd7ec9a01de74fb6c50a7cc Mon Sep 17 00:00:00 2001 From: Pauan Date: Thu, 22 Feb 2018 15:46:39 -1000 Subject: [PATCH 06/13] Adding in documentation stubs --- src/lib.rs | 1 + src/webcore/cancel.rs | 5 +++++ src/webcore/promise.rs | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 28fbf8ef..f94e967f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -166,6 +166,7 @@ pub use webcore::number::Number; pub use webcore::object::Object; pub use webcore::array::Array; pub use webcore::symbol::Symbol; +pub use webcore::cancel::{Cancel, AutoCancel}; pub use webcore::unsafe_typed_array::UnsafeTypedArray; pub use webcore::once::Once; diff --git a/src/webcore/cancel.rs b/src/webcore/cancel.rs index a552998c..afc10cf4 100644 --- a/src/webcore/cancel.rs +++ b/src/webcore/cancel.rs @@ -1,11 +1,14 @@ use std::ops::{Deref, DerefMut}; +/// pub trait Cancel { + /// fn cancel( &mut self ); } +/// #[must_use = " The AutoCancel is unused, which causes it to be immediately cancelled. @@ -21,11 +24,13 @@ pub trait Cancel { pub struct AutoCancel< A: Cancel >( Option< A > ); impl< A: Cancel > AutoCancel< A > { + /// #[inline] pub fn new( canceler: A ) -> Self { AutoCancel( Some( canceler ) ) } + /// #[inline] pub fn leak( mut self ) -> A { self.0.take().unwrap() diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index 58953c4e..e909ab7f 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -166,7 +166,7 @@ impl Promise { /// no longer interested in the `Promise`'s result. /// /// But if you *are* interested in the `Promise`'s result, then you either need to make sure to keep the handle - /// alive until after the callback is called, or you need to use the `leak` method. + /// alive until after the callback is called, or you need to use the [`leak`](struct.AutoCancel.html#method.leak) method. /// /// Cancelling the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises /// do not support cancellation. From 65e471b48ea2c6998fbe142c57b96b078d68e3c4 Mon Sep 17 00:00:00 2001 From: Pauan Date: Fri, 23 Feb 2018 22:05:27 -1000 Subject: [PATCH 07/13] Renaming AutoCancel to CancelOnDrop, also reverting most changes --- src/lib.rs | 4 ++-- src/webapi/event_target.rs | 17 ++++++++--------- src/webapi/mutation_observer.rs | 13 ++++++------- src/webapi/window.rs | 9 ++++----- src/webcore/cancel.rs | 29 ++++++++++++++++------------- src/webcore/promise.rs | 8 ++++---- src/webcore/promise_future.rs | 4 ++-- 7 files changed, 42 insertions(+), 42 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f94e967f..9affe8f1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -166,7 +166,7 @@ pub use webcore::number::Number; pub use webcore::object::Object; pub use webcore::array::Array; pub use webcore::symbol::Symbol; -pub use webcore::cancel::{Cancel, AutoCancel}; +pub use webcore::cancel::{Cancel, CancelOnDrop}; pub use webcore::unsafe_typed_array::UnsafeTypedArray; pub use webcore::once::Once; @@ -208,7 +208,7 @@ pub mod web { }; pub use webapi::cross_origin_setting::CrossOriginSetting; pub use webapi::date::Date; - pub use webapi::event_target::{IEventTarget, EventTarget, EventListener}; + pub use webapi::event_target::{IEventTarget, EventTarget, EventListenerHandle}; pub use webapi::window::RequestAnimationFrameHandle; pub use webapi::node::{INode, Node, CloneKind}; pub use webapi::element::{IElement, Element}; diff --git a/src/webapi/event_target.rs b/src/webapi/event_target.rs index 5d2da66c..049c54e6 100644 --- a/src/webapi/event_target.rs +++ b/src/webapi/event_target.rs @@ -4,29 +4,28 @@ use webcore::value::Reference; use webcore::try_from::TryInto; use webcore::reference_type::ReferenceType; use webapi::event::{ConcreteEvent, IEvent}; -use webcore::cancel::{Cancel, AutoCancel}; use private::TODO; /// A handle to a particular event listener. -pub struct EventListener { +pub struct EventListenerHandle { event_type: &'static str, reference: Reference, listener_reference: Reference } -impl fmt::Debug for EventListener { +impl fmt::Debug for EventListenerHandle { fn fmt( &self, formatter: &mut fmt::Formatter ) -> fmt::Result { - write!( formatter, "EventListener {{ event_type: {}, reference: {:?} }}", self.event_type, self.reference ) + write!( formatter, "EventListenerHandle {{ event_type: {}, reference: {:?} }}", self.event_type, self.reference ) } } -impl Cancel for EventListener { +impl EventListenerHandle { /// Removes the listener from the [IEventTarget](trait.IEventTarget.html) on /// which it was previously registered. /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/removeEventListener) // https://dom.spec.whatwg.org/#ref-for-dom-eventtarget-removeeventlistener%E2%91%A0 - fn cancel( &mut self ) { + pub fn remove( self ) { js! { @(no_return) var listener = @{&self.listener_reference}; @{&self.reference}.removeEventListener( @{self.event_type}, listener ); @@ -46,7 +45,7 @@ pub trait IEventTarget: ReferenceType { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener) // https://dom.spec.whatwg.org/#ref-for-dom-eventtarget-addeventlistener%E2%91%A0 - fn add_event_listener< T, F >( &self, listener: F ) -> AutoCancel< EventListener > + fn add_event_listener< T, F >( &self, listener: F ) -> EventListenerHandle where T: ConcreteEvent, F: FnMut( T ) + 'static { let reference = self.as_ref(); @@ -57,11 +56,11 @@ pub trait IEventTarget: ReferenceType { return listener; }.try_into().unwrap(); - AutoCancel::new( EventListener { + EventListenerHandle { event_type: T::EVENT_TYPE, reference: reference.clone(), listener_reference: listener_reference - } ) + } } /// Dispatches an `Event` at this `EventTarget`, invoking the affected event listeners in the diff --git a/src/webapi/mutation_observer.rs b/src/webapi/mutation_observer.rs index 002bf44b..f07e1784 100644 --- a/src/webapi/mutation_observer.rs +++ b/src/webapi/mutation_observer.rs @@ -3,7 +3,6 @@ use webcore::value::{Reference, Value, ConversionError}; use webapi::node_list::NodeList; use webcore::try_from::{TryFrom, TryInto}; use webapi::node::{INode, Node}; -use webcore::cancel::{Cancel, AutoCancel}; use private::TODO; /// Provides a way to receive notifications about changes to the DOM. @@ -63,17 +62,17 @@ impl MutationObserver { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver#Constructor) // https://dom.spec.whatwg.org/#ref-for-dom-mutationobserver-mutationobserver - pub fn new< F >( callback: F ) -> AutoCancel< MutationObserverHandle > + pub fn new< F >( callback: F ) -> MutationObserverHandle where F: FnMut( Vec< MutationRecord >, Self ) + 'static { let callback_reference: Reference = js! ( return @{callback}; ).try_into().unwrap(); - AutoCancel::new( MutationObserverHandle { + MutationObserverHandle { callback_reference: callback_reference.clone(), mutation_observer: js! ( return new MutationObserver( @{callback_reference} ); ).try_into().unwrap(), - } ) + } } /// Starts observing changes to the `target`. @@ -160,7 +159,7 @@ impl MutationObserver { /// This is created by the [`MutationObserver::new`](struct.MutationObserver.html#method.new) method, and /// it can use the same methods as [`MutationObserver`](struct.MutationObserver.html). /// -/// When the `MutationObserverHandle` is cancelled, the [`disconnect`](#method.disconnect) +/// When the `MutationObserverHandle` is dropped, the [`disconnect`](#method.disconnect) /// method will automatically be called. #[ derive( Debug ) ] pub struct MutationObserverHandle { @@ -177,9 +176,9 @@ impl std::ops::Deref for MutationObserverHandle { } } -impl Cancel for MutationObserverHandle { +impl Drop for MutationObserverHandle { #[inline] - fn cancel( &mut self ) { + fn drop( &mut self ) { self.disconnect(); js! { @(no_return) diff --git a/src/webapi/window.rs b/src/webapi/window.rs index a2facf5a..3b1d49ad 100644 --- a/src/webapi/window.rs +++ b/src/webapi/window.rs @@ -7,17 +7,16 @@ use webapi::location::Location; use webapi::history::History; use webcore::once::Once; use webcore::value::Value; -use webcore::cancel::{Cancel, AutoCancel}; /// A handle to a pending animation frame request. #[derive(Debug)] pub struct RequestAnimationFrameHandle(Value); -impl Cancel for RequestAnimationFrameHandle { +impl RequestAnimationFrameHandle { /// Cancels an animation frame request. /// /// [(Javascript docs)](https://developer.mozilla.org/en-US/docs/Web/API/Window/cancelAnimationFrame) - fn cancel( &mut self ) { + pub fn cancel( self ) { js! { @(no_return) var val = @{&self.0}; val.window.cancelAnimationFrame(val.request); @@ -124,13 +123,13 @@ impl Window { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame) // https://html.spec.whatwg.org/#the-window-object:dom-window-requestanimationframe - pub fn request_animation_frame< F: FnOnce(f64) + 'static>( &self, callback: F) -> AutoCancel< RequestAnimationFrameHandle > { + pub fn request_animation_frame< F: FnOnce(f64) + 'static>( &self, callback: F) -> RequestAnimationFrameHandle { let values: Value = js!{ var callback = @{Once(callback)}; var request = @{self}.requestAnimationFrame(callback); return { request: request, callback: callback, window: @{self} }; }; - AutoCancel::new( RequestAnimationFrameHandle(values) ) + RequestAnimationFrameHandle(values) } /// Returns the global [History](struct.History.html) object, which provides methods to diff --git a/src/webcore/cancel.rs b/src/webcore/cancel.rs index afc10cf4..9a807450 100644 --- a/src/webcore/cancel.rs +++ b/src/webcore/cancel.rs @@ -11,33 +11,36 @@ pub trait Cancel { /// #[must_use = " - The AutoCancel is unused, which causes it to be immediately cancelled. + The CancelOnDrop is unused, which causes it to be immediately cancelled. You probably don't want that to happen. How to fix this: - 1) Store the AutoCancel in a variable or data structure - 2) Use .leak() which will cause it to not be cancelled (this *will* leak memory!) + 1) Store the CancelOnDrop in a variable or data structure. + 2) Use .leak() which will cause it to not be cancelled (this *will* leak memory!). See the documentation for more details. "] #[derive(Debug)] -pub struct AutoCancel< A: Cancel >( Option< A > ); +pub struct CancelOnDrop< A: Cancel >( Option< A > ); -impl< A: Cancel > AutoCancel< A > { +impl< A: Cancel > CancelOnDrop< A > { /// #[inline] pub fn new( canceler: A ) -> Self { - AutoCancel( Some( canceler ) ) + CancelOnDrop( Some( canceler ) ) } /// #[inline] pub fn leak( mut self ) -> A { - self.0.take().unwrap() + match self.0.take() { + Some(value) => value, + None => unreachable!(), + } } } -impl< A: Cancel > Drop for AutoCancel< A > { +impl< A: Cancel > Drop for CancelOnDrop< A > { #[inline] fn drop( &mut self ) { match self.0 { @@ -47,7 +50,7 @@ impl< A: Cancel > Drop for AutoCancel< A > { } } -impl< A: Cancel > Deref for AutoCancel< A > { +impl< A: Cancel > Deref for CancelOnDrop< A > { type Target = A; #[inline] @@ -59,7 +62,7 @@ impl< A: Cancel > Deref for AutoCancel< A > { } } -impl< A: Cancel > DerefMut for AutoCancel< A > { +impl< A: Cancel > DerefMut for CancelOnDrop< A > { #[inline] fn deref_mut( &mut self ) -> &mut Self::Target { match self.0 { @@ -72,13 +75,13 @@ impl< A: Cancel > DerefMut for AutoCancel< A > { #[cfg(test)] mod tests { - use super::{Cancel, AutoCancel}; + use super::{Cancel, CancelOnDrop}; struct Foo( bool ); impl Foo { - fn new() -> AutoCancel< Foo > { - AutoCancel::new( Foo( false ) ) + fn new() -> CancelOnDrop< Foo > { + CancelOnDrop::new( Foo( false ) ) } } diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index e909ab7f..e7973f9f 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -2,7 +2,7 @@ use std; use webcore::once::Once; use webcore::value::{Value, Reference}; use webcore::try_from::{TryInto, TryFrom}; -use webcore::cancel::{Cancel, AutoCancel}; +use webcore::cancel::{Cancel, CancelOnDrop}; #[cfg(feature = "futures")] use webcore::serialization::JsSerialize; @@ -166,7 +166,7 @@ impl Promise { /// no longer interested in the `Promise`'s result. /// /// But if you *are* interested in the `Promise`'s result, then you either need to make sure to keep the handle - /// alive until after the callback is called, or you need to use the [`leak`](struct.AutoCancel.html#method.leak) method. + /// alive until after the callback is called, or you need to use the [`leak`](struct.CancelOnDrop.html#method.leak) method. /// /// Cancelling the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises /// do not support cancellation. @@ -184,7 +184,7 @@ impl Promise { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then) // https://www.ecma-international.org/ecma-262/6.0/#sec-performpromisethen - pub fn done< A, B, F >( &self, callback: F ) -> AutoCancel< DoneHandle > + pub fn done< A, B, F >( &self, callback: F ) -> CancelOnDrop< DoneHandle > where A: TryFrom< Value >, B: TryFrom< Value >, // TODO these Debug constraints are only needed because of unwrap @@ -227,7 +227,7 @@ impl Promise { return done; ); - AutoCancel::new( DoneHandle { + CancelOnDrop::new( DoneHandle { callback, done, } ) diff --git a/src/webcore/promise_future.rs b/src/webcore/promise_future.rs index 78a333f8..f25899f5 100644 --- a/src/webcore/promise_future.rs +++ b/src/webcore/promise_future.rs @@ -5,7 +5,7 @@ use webapi::error; use futures::{Future, Poll, Async}; use futures::unsync::oneshot::Receiver; use webcore::promise_executor::spawn; -use webcore::cancel::AutoCancel; +use webcore::cancel::CancelOnDrop; use super::promise::{Promise, DoneHandle}; @@ -22,7 +22,7 @@ use super::promise::{Promise, DoneHandle}; /// ``` pub struct PromiseFuture< Value, Error = error::Error > { pub(crate) future: Receiver< Result< Value, Error > >, - pub(crate) _done_handle: AutoCancel< DoneHandle >, + pub(crate) _done_handle: CancelOnDrop< DoneHandle >, } impl PromiseFuture< (), () > { From 104ffc6aa6b67a0dbf3306aafa4d4527f190a84a Mon Sep 17 00:00:00 2001 From: Pauan Date: Fri, 23 Feb 2018 22:33:48 -1000 Subject: [PATCH 08/13] Improving the implementation of DoneHandle --- src/webcore/promise.rs | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index e7973f9f..5d61ca41 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -22,15 +22,15 @@ use super::promise_future::PromiseFuture; /// See the documentation for [`done`](struct.Promise.html#method.done) for more information. #[derive( Debug, Clone )] pub struct DoneHandle { - callback: Value, - done: Value, + state: Value, } impl Cancel for DoneHandle { fn cancel( &mut self ) { js! { @(no_return) - @{&self.done}[0] = true; - @{&self.callback}.drop(); + var state = @{&self.state}; + state.cancelled = true; + state.callback.drop(); } } } @@ -207,29 +207,30 @@ impl Promise { callback( value ); }; - let callback = js!( return @{Once( callback )}; ); + let state = js!( + var callback = @{Once( callback )}; - let done = js!( - var callback = @{&callback}; - var done = [ false ]; + var state = { + cancelled: false, + callback: callback + }; // TODO don't swallow any errors thrown inside callback @{self}.then( function ( value ) { - if ( !done[0] ) { + if ( !state.cancelled ) { callback( value, true ); } }, function ( value ) { - if ( !done[0] ) { + if ( !state.cancelled ) { callback( value, false ); } } ); - return done; + return state; ); CancelOnDrop::new( DoneHandle { - callback, - done, + state, } ) } From 62f8f4bcc6a19f946138b7afa59d97ab8787cbe4 Mon Sep 17 00:00:00 2001 From: Pauan Date: Mon, 26 Feb 2018 23:59:26 -1000 Subject: [PATCH 09/13] Changing to use discard crate --- Cargo.toml | 1 + src/lib.rs | 3 +- src/webcore/cancel.rs | 98 ----------------------------------- src/webcore/mod.rs | 1 - src/webcore/promise.rs | 20 ++++--- src/webcore/promise_future.rs | 4 +- 6 files changed, 17 insertions(+), 110 deletions(-) delete mode 100644 src/webcore/cancel.rs diff --git a/Cargo.toml b/Cargo.toml index 56336900..229aed78 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ description = "A standard library for the client-side Web" build = "build.rs" [dependencies] +discard = "1.0.3" serde = { version = "1", optional = true } serde_json = { version = "1", optional = true } futures = { version = "0.1.18", optional = true } diff --git a/src/lib.rs b/src/lib.rs index 9affe8f1..61d31981 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -141,6 +141,8 @@ extern crate futures; #[macro_use] extern crate stdweb_derive; +extern crate discard; + #[macro_use] mod webcore; mod webapi; @@ -166,7 +168,6 @@ pub use webcore::number::Number; pub use webcore::object::Object; pub use webcore::array::Array; pub use webcore::symbol::Symbol; -pub use webcore::cancel::{Cancel, CancelOnDrop}; pub use webcore::unsafe_typed_array::UnsafeTypedArray; pub use webcore::once::Once; diff --git a/src/webcore/cancel.rs b/src/webcore/cancel.rs deleted file mode 100644 index 9a807450..00000000 --- a/src/webcore/cancel.rs +++ /dev/null @@ -1,98 +0,0 @@ -use std::ops::{Deref, DerefMut}; - - -/// -pub trait Cancel { - /// - fn cancel( &mut self ); -} - - -/// -#[must_use = " - - The CancelOnDrop is unused, which causes it to be immediately cancelled. - You probably don't want that to happen. - - How to fix this: - 1) Store the CancelOnDrop in a variable or data structure. - 2) Use .leak() which will cause it to not be cancelled (this *will* leak memory!). - - See the documentation for more details. -"] -#[derive(Debug)] -pub struct CancelOnDrop< A: Cancel >( Option< A > ); - -impl< A: Cancel > CancelOnDrop< A > { - /// - #[inline] - pub fn new( canceler: A ) -> Self { - CancelOnDrop( Some( canceler ) ) - } - - /// - #[inline] - pub fn leak( mut self ) -> A { - match self.0.take() { - Some(value) => value, - None => unreachable!(), - } - } -} - -impl< A: Cancel > Drop for CancelOnDrop< A > { - #[inline] - fn drop( &mut self ) { - match self.0 { - Some( ref mut canceler ) => canceler.cancel(), - None => {}, - } - } -} - -impl< A: Cancel > Deref for CancelOnDrop< A > { - type Target = A; - - #[inline] - fn deref( &self ) -> &Self::Target { - match self.0 { - Some( ref canceler ) => canceler, - None => unreachable!(), - } - } -} - -impl< A: Cancel > DerefMut for CancelOnDrop< A > { - #[inline] - fn deref_mut( &mut self ) -> &mut Self::Target { - match self.0 { - Some( ref mut canceler ) => canceler, - None => unreachable!(), - } - } -} - - -#[cfg(test)] -mod tests { - use super::{Cancel, CancelOnDrop}; - - struct Foo( bool ); - - impl Foo { - fn new() -> CancelOnDrop< Foo > { - CancelOnDrop::new( Foo( false ) ) - } - } - - impl Cancel for Foo { - fn cancel( &mut self ) { - self.0 = true; - } - } - - #[test] - fn unused() { - Foo::new(); - } -} diff --git a/src/webcore/mod.rs b/src/webcore/mod.rs index 8b26b51e..7088fb94 100644 --- a/src/webcore/mod.rs +++ b/src/webcore/mod.rs @@ -17,7 +17,6 @@ pub mod once; pub mod instance_of; pub mod reference_type; pub mod promise; -pub mod cancel; #[cfg(feature = "futures")] pub mod promise_future; diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index 5d61ca41..912955b9 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -2,7 +2,7 @@ use std; use webcore::once::Once; use webcore::value::{Value, Reference}; use webcore::try_from::{TryInto, TryFrom}; -use webcore::cancel::{Cancel, CancelOnDrop}; +use discard::{Discard, DiscardOnDrop}; #[cfg(feature = "futures")] use webcore::serialization::JsSerialize; @@ -25,8 +25,8 @@ pub struct DoneHandle { state: Value, } -impl Cancel for DoneHandle { - fn cancel( &mut self ) { +impl Discard for DoneHandle { + fn discard( self ) { js! { @(no_return) var state = @{&self.state}; state.cancelled = true; @@ -162,13 +162,17 @@ impl Promise { /// If the `Promise` never succeeds / fails then the `callback` will never be called. /// /// This method returns a [`DoneHandle`](struct.DoneHandle.html). When the [`DoneHandle`](struct.DoneHandle.html) - /// is cancelled it will drop the `callback` and the `callback` will never be called. This is useful if you are + /// is dropped it will drop the `callback` and the `callback` will never be called. This is useful if you are /// no longer interested in the `Promise`'s result. /// /// But if you *are* interested in the `Promise`'s result, then you either need to make sure to keep the handle - /// alive until after the callback is called, or you need to use the [`leak`](struct.CancelOnDrop.html#method.leak) method. + /// alive until after the callback is called, or you need to use the [`leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) + /// function. /// - /// Cancelling the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises + /// If you choose to leak the [`DoneHandle`](struct.DoneHandle.html) then it ***will*** leak the memory for the + /// callback, so only do that if absolutely need to. + /// + /// Discarding the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises /// do not support cancellation. /// /// # Examples @@ -184,7 +188,7 @@ impl Promise { /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then) // https://www.ecma-international.org/ecma-262/6.0/#sec-performpromisethen - pub fn done< A, B, F >( &self, callback: F ) -> CancelOnDrop< DoneHandle > + pub fn done< A, B, F >( &self, callback: F ) -> DiscardOnDrop< DoneHandle > where A: TryFrom< Value >, B: TryFrom< Value >, // TODO these Debug constraints are only needed because of unwrap @@ -229,7 +233,7 @@ impl Promise { return state; ); - CancelOnDrop::new( DoneHandle { + DiscardOnDrop::new( DoneHandle { state, } ) } diff --git a/src/webcore/promise_future.rs b/src/webcore/promise_future.rs index f25899f5..4097c5c3 100644 --- a/src/webcore/promise_future.rs +++ b/src/webcore/promise_future.rs @@ -5,7 +5,7 @@ use webapi::error; use futures::{Future, Poll, Async}; use futures::unsync::oneshot::Receiver; use webcore::promise_executor::spawn; -use webcore::cancel::CancelOnDrop; +use discard::DiscardOnDrop; use super::promise::{Promise, DoneHandle}; @@ -22,7 +22,7 @@ use super::promise::{Promise, DoneHandle}; /// ``` pub struct PromiseFuture< Value, Error = error::Error > { pub(crate) future: Receiver< Result< Value, Error > >, - pub(crate) _done_handle: CancelOnDrop< DoneHandle >, + pub(crate) _done_handle: DiscardOnDrop< DoneHandle >, } impl PromiseFuture< (), () > { From 1f0568c24ef5e055da13097d71ac3fbc89e5a244 Mon Sep 17 00:00:00 2001 From: Pauan Date: Tue, 27 Feb 2018 00:48:18 -1000 Subject: [PATCH 10/13] Improving the docs for done --- src/webcore/promise.rs | 40 ++++++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index 912955b9..939b2b18 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -161,22 +161,29 @@ impl Promise { /// /// If the `Promise` never succeeds / fails then the `callback` will never be called. /// - /// This method returns a [`DoneHandle`](struct.DoneHandle.html). When the [`DoneHandle`](struct.DoneHandle.html) - /// is dropped it will drop the `callback` and the `callback` will never be called. This is useful if you are - /// no longer interested in the `Promise`'s result. + /// This method returns a [`DoneHandle`](struct.DoneHandle.html). The [`DoneHandle`](struct.DoneHandle.html) + /// *exclusively* owns the `callback`, so when the [`DoneHandle`](struct.DoneHandle.html) is dropped it will + /// drop the `callback` and the `callback` will never be called. This will happen even if the `Promise` is not dropped! /// - /// But if you *are* interested in the `Promise`'s result, then you either need to make sure to keep the handle - /// alive until after the callback is called, or you need to use the [`leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) - /// function. + /// Dropping the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises + /// do not support cancellation. /// - /// If you choose to leak the [`DoneHandle`](struct.DoneHandle.html) then it ***will*** leak the memory for the - /// callback, so only do that if absolutely need to. + /// If you are no longer interested in the `Promise`'s result you can simply drop the [`DoneHandle`](struct.DoneHandle.html) + /// and then the `callback` will never be called. /// - /// Discarding the [`DoneHandle`](struct.DoneHandle.html) does ***not*** cancel the `Promise`, because promises - /// do not support cancellation. + /// But if you *are* interested in the `Promise`'s result, then you have two choices: + /// + /// * Keep the [`DoneHandle`](struct.DoneHandle.html) alive until after the `callback` is called (by storing it in a + /// variable or data structure). + /// + /// * Use the [`leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) function to leak the + /// [`DoneHandle`](struct.DoneHandle.html). This ***will*** leak the memory of the callback, so only do that if you + /// absolutely need to. /// /// # Examples /// + /// Normal usage: + /// /// ```rust /// let handle = promise.done(|result| { /// match result { @@ -186,6 +193,19 @@ impl Promise { /// }); /// ``` /// + /// Leak the [`DoneHandle`](struct.DoneHandle.html) and `callback`: + /// + /// ```rust + /// use discard::DiscardOnDrop; + /// + /// DiscardOnDrop::leak(promise.done(|result| { + /// match result { + /// Ok(success) => { ... }, + /// Err(error) => { ... }, + /// } + /// })); + /// ``` + /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then) // https://www.ecma-international.org/ecma-262/6.0/#sec-performpromisethen pub fn done< A, B, F >( &self, callback: F ) -> DiscardOnDrop< DoneHandle > From e1d9b4c4cd3c3f3d8ebf48df0fc4828181ebce85 Mon Sep 17 00:00:00 2001 From: Pauan Date: Tue, 27 Feb 2018 01:03:17 -1000 Subject: [PATCH 11/13] Minor improvement to the docs --- src/webcore/promise.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index 939b2b18..d834119b 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -177,8 +177,9 @@ impl Promise { /// variable or data structure). /// /// * Use the [`leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) function to leak the - /// [`DoneHandle`](struct.DoneHandle.html). This ***will*** leak the memory of the callback, so only do that if you - /// absolutely need to. + /// [`DoneHandle`](struct.DoneHandle.html). If the `Promise` never succeeds or fails then this ***will*** leak the + /// memory of the callback, so only use [`leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) + /// if you need to. /// /// # Examples /// From e6459882623fff4d92d9d25a9803c7ecdbb3c482 Mon Sep 17 00:00:00 2001 From: Pauan Date: Tue, 27 Feb 2018 01:10:58 -1000 Subject: [PATCH 12/13] Minor improvement to the docs --- src/webcore/promise.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index d834119b..03a29b81 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -176,9 +176,9 @@ impl Promise { /// * Keep the [`DoneHandle`](struct.DoneHandle.html) alive until after the `callback` is called (by storing it in a /// variable or data structure). /// - /// * Use the [`leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) function to leak the + /// * Use the [`DiscardOnDrop::leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) function to leak the /// [`DoneHandle`](struct.DoneHandle.html). If the `Promise` never succeeds or fails then this ***will*** leak the - /// memory of the callback, so only use [`leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) + /// memory of the callback, so only use [`DiscardOnDrop::leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) /// if you need to. /// /// # Examples From 84a95e81947b7a5279125f6243fede4d2ca9d023 Mon Sep 17 00:00:00 2001 From: Pauan Date: Thu, 22 Mar 2018 04:10:05 -1000 Subject: [PATCH 13/13] Adding in custom DiscardOnDrop struct --- src/lib.rs | 2 + src/webcore/discard.rs | 160 ++++++++++++++++++++++++++++++++++ src/webcore/mod.rs | 1 + src/webcore/promise.rs | 16 ++-- src/webcore/promise_future.rs | 2 +- 5 files changed, 171 insertions(+), 10 deletions(-) create mode 100644 src/webcore/discard.rs diff --git a/src/lib.rs b/src/lib.rs index 61d31981..df16f441 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -175,6 +175,8 @@ pub use webcore::instance_of::InstanceOf; pub use webcore::reference_type::ReferenceType; pub use webcore::serialization::JsSerialize; +pub use webcore::discard::DiscardOnDrop; + #[cfg(feature = "experimental_features_which_may_break_on_minor_version_bumps")] pub use webcore::promise::{Promise, DoneHandle}; diff --git a/src/webcore/discard.rs b/src/webcore/discard.rs new file mode 100644 index 00000000..4c2e9f3c --- /dev/null +++ b/src/webcore/discard.rs @@ -0,0 +1,160 @@ +use discard; +use discard::Discard; +use std::ops::{Deref, DerefMut}; + + +/// If you have a value which implements [`Discard`](https://docs.rs/discard/%5E1.0.3/discard/trait.Discard.html), you can use +/// [`DiscardOnDrop::new(value)`](struct.DiscardOnDrop.html#method.new) which will wrap the value. +/// When the wrapper is dropped it will automatically call [`value.discard()`](https://docs.rs/discard/%5E1.0.3/discard/trait.Discard.html#tymethod.discard). +/// +/// You can use the [`leak`](#method.leak) method to unwrap it (which returns `value`). This causes +/// it to no longer call [`discard`](https://docs.rs/discard/%5E1.0.3/discard/trait.Discard.html#tymethod.discard) when it is dropped, which +/// means it will usually leak memory unless you manually call [`discard`](https://docs.rs/discard/%5E1.0.3/discard/trait.Discard.html#tymethod.discard). +#[must_use = " + + The DiscardOnDrop is unused, which causes it to be immediately discarded. + You probably don't want that to happen. + + How to fix this: + + * Store the DiscardOnDrop in a variable or data structure. + + * Or use the leak() method which will cause it to not be + discarded (this will usually leak memory!) + + See the documentation for more details. +"] +#[derive(Debug)] +pub struct DiscardOnDrop< A: Discard >( discard::DiscardOnDrop< A > ); + +impl< A: Discard > DiscardOnDrop< A > { + /// Creates a new `DiscardOnDrop`. + /// + /// When the `DiscardOnDrop` is dropped it will automatically call [`discarder.discard()`](https://docs.rs/discard/%5E1.0.3/discard/trait.Discard.html#tymethod.discard). + #[inline] + pub fn new( discarder: A ) -> Self { + DiscardOnDrop( discard::DiscardOnDrop::new( discarder ) ) + } + + /// Returns the wrapped `discarder`. + /// + /// It will no longer automatically call [`discarder.discard()`](https://docs.rs/discard/%5E1.0.3/discard/trait.Discard.html#tymethod.discard), so this will usually leak + /// memory unless you manually call [`discarder.discard()`](https://docs.rs/discard/%5E1.0.3/discard/trait.Discard.html#tymethod.discard). + #[inline] + pub fn leak( self ) -> A { + discard::DiscardOnDrop::leak( self.0 ) + } +} + +impl< A: Discard > Deref for DiscardOnDrop< A > { + type Target = A; + + #[inline] + fn deref( &self ) -> &Self::Target { + &*self.0 + } +} + +impl< A: Discard > DerefMut for DiscardOnDrop< A > { + #[inline] + fn deref_mut( &mut self ) -> &mut Self::Target { + &mut *self.0 + } +} + + +#[cfg(test)] +mod tests { + use discard::Discard; + use super::DiscardOnDrop; + use std::rc::Rc; + use std::cell::Cell; + + struct Foo( Rc< Cell< bool > > ); + + impl Foo { + fn new() -> Self { + Foo( Rc::new( Cell::new( false ) ) ) + } + + fn dropped( &self ) -> Rc< Cell< bool > > { + self.0.clone() + } + + fn as_mut( &mut self ) -> &mut Self { + self + } + } + + impl Discard for Foo { + fn discard( self ) { + self.0.set( true ); + } + } + + + #[test] + fn unused() { + Foo::new(); + } + + #[test] + fn unused_discard_on_drop() { + DiscardOnDrop::new( Foo::new() ); + } + + #[test] + fn discard() { + let foo = Foo::new(); + + let dropped = foo.dropped(); + + assert_eq!( dropped.get(), false ); + foo.discard(); + assert_eq!( dropped.get(), true ); + } + + #[test] + fn no_discard() { + let foo = Foo::new(); + + let dropped = foo.dropped(); + + assert_eq!( dropped.get(), false ); + drop( foo ); + assert_eq!( dropped.get(), false ); + } + + #[test] + fn discard_on_drop() { + let foo = DiscardOnDrop::new( Foo::new() ); + + let dropped = foo.dropped(); + + assert_eq!( dropped.get(), false ); + drop( foo ); + assert_eq!( dropped.get(), true ); + } + + #[test] + fn leak() { + let foo = DiscardOnDrop::new(Foo::new()); + + let dropped = foo.dropped(); + + assert_eq!( dropped.get(), false ); + drop( foo.leak() ); + assert_eq!( dropped.get(), false ); + } + + #[test] + fn deref_mut() { + let mut foo = DiscardOnDrop::new( Foo::new() ); + + let dropped = foo.as_mut().dropped(); + + assert_eq!( dropped.get(), false ); + drop( foo.leak() ); + assert_eq!( dropped.get(), false ); + } +} diff --git a/src/webcore/mod.rs b/src/webcore/mod.rs index 7088fb94..e81ae928 100644 --- a/src/webcore/mod.rs +++ b/src/webcore/mod.rs @@ -17,6 +17,7 @@ pub mod once; pub mod instance_of; pub mod reference_type; pub mod promise; +pub mod discard; #[cfg(feature = "futures")] pub mod promise_future; diff --git a/src/webcore/promise.rs b/src/webcore/promise.rs index 03a29b81..ce10b5b2 100644 --- a/src/webcore/promise.rs +++ b/src/webcore/promise.rs @@ -1,8 +1,9 @@ use std; +use discard::Discard; use webcore::once::Once; use webcore::value::{Value, Reference}; use webcore::try_from::{TryInto, TryFrom}; -use discard::{Discard, DiscardOnDrop}; +use webcore::discard::DiscardOnDrop; #[cfg(feature = "futures")] use webcore::serialization::JsSerialize; @@ -176,10 +177,9 @@ impl Promise { /// * Keep the [`DoneHandle`](struct.DoneHandle.html) alive until after the `callback` is called (by storing it in a /// variable or data structure). /// - /// * Use the [`DiscardOnDrop::leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) function to leak the - /// [`DoneHandle`](struct.DoneHandle.html). If the `Promise` never succeeds or fails then this ***will*** leak the - /// memory of the callback, so only use [`DiscardOnDrop::leak`](https://docs.rs/discard/1.*/discard/struct.DiscardOnDrop.html#method.leak) - /// if you need to. + /// * Use the [`leak`](struct.DiscardOnDrop.html#method.leak) method to leak the [`DoneHandle`](struct.DoneHandle.html). + /// If the `Promise` never succeeds or fails then this ***will*** leak the memory of the callback, so only use + /// [`leak`](struct.DiscardOnDrop.html#method.leak) if you need to. /// /// # Examples /// @@ -197,14 +197,12 @@ impl Promise { /// Leak the [`DoneHandle`](struct.DoneHandle.html) and `callback`: /// /// ```rust - /// use discard::DiscardOnDrop; - /// - /// DiscardOnDrop::leak(promise.done(|result| { + /// promise.done(|result| { /// match result { /// Ok(success) => { ... }, /// Err(error) => { ... }, /// } - /// })); + /// }).leak(); /// ``` /// /// [(JavaScript docs)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then) diff --git a/src/webcore/promise_future.rs b/src/webcore/promise_future.rs index 4097c5c3..40fb8494 100644 --- a/src/webcore/promise_future.rs +++ b/src/webcore/promise_future.rs @@ -5,7 +5,7 @@ use webapi::error; use futures::{Future, Poll, Async}; use futures::unsync::oneshot::Receiver; use webcore::promise_executor::spawn; -use discard::DiscardOnDrop; +use webcore::discard::DiscardOnDrop; use super::promise::{Promise, DoneHandle};