Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Promise memory leak #127

Merged
merged 13 commits into from
Mar 22, 2018
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
6 changes: 5 additions & 1 deletion src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ extern crate futures;
#[macro_use]
extern crate stdweb_derive;

extern crate discard;

#[macro_use]
mod webcore;
mod webapi;
Expand Down Expand Up @@ -173,8 +175,10 @@ 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;
pub use webcore::promise::{Promise, DoneHandle};

#[cfg(all(
feature = "futures",
Expand Down
11 changes: 5 additions & 6 deletions src/webapi/event_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,16 @@ impl fmt::Debug for EventListenerHandle {
}

impl EventListenerHandle {
/// Removes the handler from the [IEventTarget](trait.IEventTarget.html) on
/// 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 ) {
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 );
}
}
}
Expand All @@ -42,7 +40,7 @@ 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)
Expand All @@ -51,6 +49,7 @@ pub trait IEventTarget: ReferenceType {
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 );
Expand Down
2 changes: 1 addition & 1 deletion src/webapi/mutation_observer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ impl std::ops::Deref for MutationObserverHandle {
type Target = MutationObserver;

#[inline]
fn deref(&self) -> &Self::Target {
fn deref( &self ) -> &Self::Target {
&self.mutation_observer
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/webapi/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,12 @@ impl 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};
pub fn cancel( self ) {
js! { @(no_return)
var val = @{&self.0};
val.window.cancelAnimationFrame(val.request);
val.callback.drop();
};
}
}
}

Expand Down
160 changes: 160 additions & 0 deletions src/webcore/discard.rs
Original file line number Diff line number Diff line change
@@ -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 );
}
}
1 change: 1 addition & 0 deletions src/webcore/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Loading