Eliminate box indirection in resources. Fixes #40. (#73)

* Eliminate box indirection in resources. Fixes #40.

* Add tests for resource cleanup.

Author: jorendorff

src/codegen_runtime.rs

@@ -66,7 +66,7 @@ pub unsafe fn handle_nif_init_call(function: Option<for<'a> fn(NifEnv<'a>, NifTe
 use std;
 use ::resource::align_alloced_mem_for_struct;
 pub unsafe fn handle_drop_resource_struct_handle<T: NifResourceTypeProvider>(_env: NIF_ENV, handle: MUTABLE_NIF_RESOURCE_HANDLE) {
-    let aligned = align_alloced_mem_for_struct::<Box<T>>(handle);
-    let res = aligned as *mut Box<T>;
+    let aligned = align_alloced_mem_for_struct::<T>(handle);
+    let res = aligned as *mut T;
     std::mem::drop(std::ptr::read(res));
 }

src/resource.rs

@@ -84,18 +84,18 @@ pub unsafe fn align_alloced_mem_for_struct<T>(ptr: *const c_void) -> *const c_vo
 /// resource instance on creation, and decrements when dropped.
 pub struct ResourceCell<T> where T: NifResourceTypeProvider + Sync {
     raw: *const c_void,
-    inner: *mut Box<T>,
+    inner: *mut T,
 }
 
 impl<T> ResourceCell<T> where T: NifResourceTypeProvider + Sync {
     /// Makes a new ResourceCell from the given type. Note that the type must have
     /// NifResourceTypeProvider implemented for it. See module documentation for info on this.
     pub fn new(data: T) -> Self {
-        let alloc_size = get_alloc_size_struct::<Box<T>>();
+        let alloc_size = get_alloc_size_struct::<T>();
         let mem_raw = unsafe { ::wrapper::resource::alloc_resource(T::get_type().res, alloc_size) };
-        let aligned_mem = unsafe { align_alloced_mem_for_struct::<Box<T>>(mem_raw) } as *mut Box<T>;
+        let aligned_mem = unsafe { align_alloced_mem_for_struct::<T>(mem_raw) as *mut T };
 
-        unsafe { ptr::write(aligned_mem, Box::new(data)) };
+        unsafe { ptr::write(aligned_mem, data) };
 
         ResourceCell {
             raw: mem_raw,
@@ -109,7 +109,7 @@ impl<T> ResourceCell<T> where T: NifResourceTypeProvider + Sync {
             None => return Err(NifError::BadArg),
         };
         unsafe { ::wrapper::resource::keep_resource(res_resource); }
-        let casted_ptr = unsafe { align_alloced_mem_for_struct::<Box<T>>(res_resource) } as *mut Box<T>;
+        let casted_ptr = unsafe { align_alloced_mem_for_struct::<T>(res_resource) as *mut T };
         Ok(ResourceCell {
             raw: res_resource,
             inner: casted_ptr,

test/lib/rustler_test.ex

@@ -23,6 +23,8 @@ defmodule RustlerTest do
   def resource_make(), do: err()
   def resource_set_integer_field(_, _), do: err()
   def resource_get_integer_field(_), do: err()
+  def resource_make_immutable(_), do: err()
+  def resource_immutable_count(), do: err()
 
   def make_shorter_subbinary(_), do: err()
   def parse_integer(_), do: err()

test/src/lib.in.rs

@@ -26,6 +26,8 @@ rustler_export_nifs!(
         ("resource_make", 0, test_resource::resource_make),
         ("resource_set_integer_field", 2, test_resource::resource_set_integer_field),
         ("resource_get_integer_field", 1, test_resource::resource_get_integer_field),
+        ("resource_make_immutable", 1, test_resource::resource_make_immutable),
+        ("resource_immutable_count", 0, test_resource::resource_immutable_count),
 
         ("atom_to_string", 1, test_atom::atom_to_string),
         ("atom_equals_ok", 1, test_atom::atom_equals_ok),

test/src/test_resource.rs

@@ -7,8 +7,16 @@ struct TestResource {
     test_field: RwLock<i32>,
 }
 
+/// This one is designed to look more like pointer data, to increase the
+/// chance of segfaults if the implementation is wrong.
+struct ImmutableResource {
+    a: u32,
+    b: u32
+}
+
 pub fn on_load<'a>(env: NifEnv<'a>) -> bool {
     resource_struct_init!(TestResource, env);
+    resource_struct_init!(ImmutableResource, env);
     true
 }
 
@@ -34,3 +42,39 @@ pub fn resource_get_integer_field<'a>(env: NifEnv<'a>, args: &Vec<NifTerm<'a>>)
     let test_field = resource.test_field.read().unwrap();
     Ok(test_field.encode(env))
 }
+
+
+use std::sync::atomic::{ AtomicUsize, Ordering };
+
+lazy_static! {
+    static ref COUNT: AtomicUsize = AtomicUsize::new(0);
+}
+
+impl ImmutableResource {
+    fn new(u: u32) -> ImmutableResource {
+        COUNT.fetch_add(1, Ordering::SeqCst);
+        ImmutableResource {
+            a: u,
+            b: !u
+        }
+    }
+}
+
+impl Drop for ImmutableResource {
+    fn drop(&mut self) {
+        assert_eq!(self.a, !self.b);
+        self.b = self.a;
+        COUNT.fetch_sub(1, Ordering::SeqCst);
+    }
+}
+
+pub fn resource_make_immutable<'a>(env: NifEnv<'a>, args: &Vec<NifTerm<'a>>) -> NifResult<NifTerm<'a>> {
+    let u: u32 = try!(args[0].decode());
+    Ok(ResourceCell::new(ImmutableResource::new(u)).encode(env))
+}
+
+/// Count how many instances of `ImmutableResource` are currently alive globally.
+pub fn resource_immutable_count<'a>(env: NifEnv<'a>, _args: &Vec<NifTerm<'a>>) -> NifResult<NifTerm<'a>> {
+    let n = COUNT.load(Ordering::SeqCst) as u32;
+    Ok(n.encode(env))
+}

test/test/resource_test.exs

@@ -1,11 +1,36 @@
 defmodule RustlerTest.ResourceTest do
   use ExUnit.Case, async: true
+  use Bitwise
 
   test "resource creation and interaction" do
-    resource = RustlerTest.resource_make
+    resource = RustlerTest.resource_make()
     assert resource == "" # A resource looks like an empty binary :(
     assert RustlerTest.resource_get_integer_field(resource) == 0
     RustlerTest.resource_set_integer_field(resource, 10)
     assert RustlerTest.resource_get_integer_field(resource) == 10
   end
+
+  test "resource cleanup" do
+    # Create a bunch of unreferenced resources for the GC to cleanup.
+    for i <- 0..1000 do
+      resource = RustlerTest.resource_make()
+      RustlerTest.resource_set_integer_field(resource, i)
+    end
+
+    # Clean them up. Don't crash.
+    :erlang.garbage_collect()
+  end
+
+  test "resource cleanup 2" do
+    # Create a bunch of unreferenced resources for the GC to cleanup.
+    for i <- 0..1000 do
+      RustlerTest.resource_make_immutable((i * 0x11235813) &&& 0xffffffff)
+    end
+
+    # Clean them up. Don't crash.
+    :erlang.garbage_collect()
+
+    # Erlang's exact GC should have cleaned all that up.
+    assert RustlerTest.resource_immutable_count() == 0
+  end
 end