simplify the exchange allocator

* stop using an atomic counter, this has a significant cost and
  valgrind will already catch these leaks
* remove the extra layer of function calls
* remove the assert of non-null in free, freeing null is well defined
  but throwing a failure from free will not be
* stop initializing the `prev`/`next` pointers
* abort on out-of-memory, failing won't necessarily work

Author: thestinger

src/libstd/os.rs

@@ -730,7 +730,7 @@ pub fn list_dir(p: &Path) -> ~[~str] {
         #[cfg(windows)]
         unsafe fn get_list(p: &Path) -> ~[~str] {
             use libc::consts::os::extra::INVALID_HANDLE_VALUE;
-            use libc::wcslen;
+            use libc::{wcslen, free};
             use libc::funcs::extra::kernel32::{
                 FindFirstFileW,
                 FindNextFileW,
@@ -739,7 +739,7 @@ pub fn list_dir(p: &Path) -> ~[~str] {
             use os::win32::{
                 as_utf16_p
             };
-            use rt::global_heap::{malloc_raw, free_raw};
+            use rt::global_heap::malloc_raw;
             #[nolink]
             extern {
                 unsafe fn rust_list_dir_wfd_size() -> libc::size_t;
@@ -772,7 +772,7 @@ pub fn list_dir(p: &Path) -> ~[~str] {
                             ::cast::transmute(wfd_ptr));
                     }
                     FindClose(find_handle);
-                    free_raw(wfd_ptr);
+                    free(wfd_ptr)
                 }
                 strings
             }

src/libstd/rt/global_heap.rs

@@ -8,62 +8,21 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-use sys::{size_of};
-use libc::{c_void, size_t, uintptr_t};
-use c_malloc = libc::malloc;
-use c_free = libc::free;
+use libc::{c_char, c_void, size_t, uintptr_t, free, malloc};
 use managed::raw::{BoxHeaderRepr, BoxRepr};
-use cast::transmute;
-use unstable::intrinsics::{atomic_xadd,atomic_xsub,TyDesc};
-use ptr::null;
+use unstable::intrinsics::TyDesc;
+use sys::size_of;
 
-pub unsafe fn malloc(td: *TyDesc, size: uint) -> *c_void {
-    assert!(td.is_not_null());
-
-    let total_size = get_box_size(size, (*td).align);
-    let p = c_malloc(total_size as size_t);
-    assert!(p.is_not_null());
-
-    let box: &mut BoxRepr = transmute(p);
-    box.header.ref_count = -1; // Exchange values not ref counted
-    box.header.type_desc = td;
-    box.header.prev = null();
-    box.header.next = null();
-
-    let exchange_count = &mut *exchange_count_ptr();
-    atomic_xadd(exchange_count, 1);
-
-    return transmute(box);
-}
-/**
-Thin wrapper around libc::malloc, none of the box header
-stuff in exchange_alloc::malloc
-*/
-pub unsafe fn malloc_raw(size: uint) -> *c_void {
-    let p = c_malloc(size as size_t);
-    if p.is_null() {
-        fail!("Failure in malloc_raw: result ptr is null");
-    }
-    p
-}
-
-pub unsafe fn free(ptr: *c_void) {
-    let exchange_count = &mut *exchange_count_ptr();
-    atomic_xsub(exchange_count, 1);
-
-    assert!(ptr.is_not_null());
-    c_free(ptr);
-}
-///Thin wrapper around libc::free, as with exchange_alloc::malloc_raw
-pub unsafe fn free_raw(ptr: *c_void) {
-    c_free(ptr);
+extern {
+    #[rust_stack]
+    fn abort();
 }
 
 fn get_box_size(body_size: uint, body_align: uint) -> uint {
     let header_size = size_of::<BoxHeaderRepr>();
     // FIXME (#2699): This alignment calculation is suspicious. Is it right?
     let total_size = align_to(header_size, body_align) + body_size;
-    return total_size;
+    total_size
 }
 
 // Rounds |size| to the nearest |alignment|. Invariant: |alignment| is a power
@@ -73,11 +32,40 @@ fn align_to(size: uint, align: uint) -> uint {
     (size + align - 1) & !(align - 1)
 }
 
-fn exchange_count_ptr() -> *mut int {
-    // XXX: Need mutable globals
-    unsafe { transmute(&rust_exchange_count) }
+/// A wrapper around libc::malloc, aborting on out-of-memory
+pub unsafe fn malloc_raw(size: uint) -> *c_void {
+    let p = malloc(size as size_t);
+    if p.is_null() {
+        // we need a non-allocating way to print an error here
+        abort();
+    }
+    p
 }
 
-extern {
-    static rust_exchange_count: uintptr_t;
+// FIXME #4942: Make these signatures agree with exchange_alloc's signatures
+#[lang="exchange_malloc"]
+#[inline]
+pub unsafe fn exchange_malloc(td: *c_char, size: uintptr_t) -> *c_char {
+    let td = td as *TyDesc;
+    let size = size as uint;
+
+    assert!(td.is_not_null());
+
+    let total_size = get_box_size(size, (*td).align);
+    let p = malloc_raw(total_size as uint);
+
+    let box: *mut BoxRepr = p as *mut BoxRepr;
+    (*box).header.ref_count = -1; // Exchange values not ref counted
+    (*box).header.type_desc = td;
+
+    box as *c_char
+}
+
+// NB: Calls to free CANNOT be allowed to fail, as throwing an exception from
+// inside a landing pad may corrupt the state of the exception handler.
+#[cfg(not(test))]
+#[lang="exchange_free"]
+#[inline]
+pub unsafe fn exchange_free(ptr: *c_char) {
+    free(ptr as *c_void);
 }

src/libstd/unstable/exchange_alloc.rs

@@ -22,7 +22,6 @@ use rt::task::Task;
 use rt::local::Local;
 use option::{Option, Some, None};
 use io;
-use rt::global_heap;
 
 #[allow(non_camel_case_types)]
 pub type rust_task = c_void;
@@ -150,13 +149,6 @@ unsafe fn fail_borrowed(box: *mut BoxRepr, file: *c_char, line: size_t) {
     }
 }
 
-// FIXME #4942: Make these signatures agree with exchange_alloc's signatures
-#[lang="exchange_malloc"]
-#[inline]
-pub unsafe fn exchange_malloc(td: *c_char, size: uintptr_t) -> *c_char {
-    transmute(global_heap::malloc(transmute(td), transmute(size)))
-}
-
 /// Because this code is so perf. sensitive, use a static constant so that
 /// debug printouts are compiled out most of the time.
 static ENABLE_DEBUG: bool = false;
@@ -228,15 +220,6 @@ impl DebugPrints for io::fd_t {
     }
 }
 
-// NB: Calls to free CANNOT be allowed to fail, as throwing an exception from
-// inside a landing pad may corrupt the state of the exception handler. If a
-// problem occurs, call exit instead.
-#[lang="exchange_free"]
-#[inline]
-pub unsafe fn exchange_free(ptr: *c_char) {
-    global_heap::free(transmute(ptr))
-}
-
 #[lang="malloc"]
 pub unsafe fn local_malloc(td: *c_char, size: uintptr_t) -> *c_char {
     match context() {

src/libstd/unstable/lang.rs

@@ -15,16 +15,10 @@
 #include <string.h>
 #include <stdio.h>
 
-extern uintptr_t rust_exchange_count;
-uintptr_t rust_exchange_count = 0;
-
 void *
 rust_exchange_alloc::malloc(size_t size) {
   void *value = ::malloc(size);
   assert(value);
-
-  sync::increment(rust_exchange_count);
-
   return value;
 }
 
@@ -37,15 +31,5 @@ rust_exchange_alloc::realloc(void *ptr, size_t size) {
 
 void
 rust_exchange_alloc::free(void *ptr) {
-  sync::decrement(rust_exchange_count);
   ::free(ptr);
 }
-
-void
-rust_check_exchange_count_on_exit() {
-  if (rust_exchange_count != 0) {
-    printf("exchange heap not empty on exit\n");
-    printf("%d dangling allocations\n", (int)rust_exchange_count);
-    abort();
-  }
-}

src/rt/rust_exchange_alloc.cpp

@@ -21,10 +21,4 @@ class rust_exchange_alloc {
     void free(void *mem);
 };
 
-extern "C" uintptr_t *
-rust_get_exchange_count_ptr();
-
-void
-rust_check_exchange_count_on_exit();
-
 #endif

src/rt/rust_exchange_alloc.h

@@ -211,7 +211,6 @@ rust_kernel::run() {
     assert(osmain_driver != NULL);
     osmain_driver->start_main_loop();
     sched_reaper.join();
-    rust_check_exchange_count_on_exit();
     return rval;
 }
 

src/rt/rust_kernel.cpp

@@ -196,7 +196,6 @@ rust_register_exit_function
 rust_get_global_data_ptr
 rust_inc_kernel_live_count
 rust_dec_kernel_live_count
-rust_exchange_count
 rust_get_rt_tls_key
 swap_registers
 rust_readdir