rt: Fix the atomic get_ref_count method to avoid races

Author: brson

src/libcore/comm.rs

@@ -44,7 +44,6 @@ native mod rustrt {
     fn del_port(po: *rust_port);
     fn rust_port_begin_detach(po: *rust_port,
                               yield: *ctypes::uintptr_t);
-    fn rust_port_end_detach(po: *rust_port);
     fn get_port_id(po: *rust_port) -> port_id;
     fn rust_port_size(po: *rust_port) -> ctypes::size_t;
     fn port_recv(dptr: *uint, po: *rust_port,
@@ -93,7 +92,6 @@ resource port_ptr<T: send>(po: *rust_port) {
         // in a bogus state.
         task::yield();
     }
-    rustrt::rust_port_end_detach(po);
 
     // Drain the port so that all the still-enqueued items get dropped
     while rustrt::rust_port_size(po) > 0u {

src/rt/rust_builtin.cpp

@@ -488,16 +488,12 @@ rust_port_begin_detach(rust_port *port, uintptr_t *yield) {
     port->begin_detach(yield);
 }
 
-extern "C" CDECL void
-rust_port_end_detach(rust_port *port) {
-    port->end_detach();
-}
-
 extern "C" CDECL void
 del_port(rust_port *port) {
     rust_task *task = rust_task_thread::get_task();
     LOG(task, comm, "del_port(0x%" PRIxPTR ")", (uintptr_t) port);
     A(task->thread, port->get_ref_count() == 0, "Expected port ref_count == 0");
+    port->end_detach();
     delete port;
 }
 

src/rt/rust_internal.h

@@ -113,11 +113,7 @@ public:                                                                    \
        assert(old > 0);                                                    \
    }                                                                       \
    void deref() { if(0 == sync::decrement(ref_count)) { delete_this(); } } \
-   intptr_t get_ref_count() {                                              \
-      sync::increment(ref_count);                                          \
-      intptr_t current = sync::decrement(ref_count);                       \
-      return current;                                                      \
-   }
+   intptr_t get_ref_count() { return sync::read(ref_count); }
 
 template <typename T> struct task_owned {
     inline void *operator new(size_t size, rust_task *task, const char *tag);

src/rt/rust_port.cpp

@@ -44,13 +44,6 @@ void rust_port::begin_detach(uintptr_t *yield) {
 }
 
 void rust_port::end_detach() {
-    // FIXME: For some reason, on rare occasion we can get here without
-    // actually having the ref count go to 0. Possibly related to #1923
-    bool done = false;
-    while (!done) {
-        done = get_ref_count() == 0;
-    }
-
     // Just take the lock to make sure that the thread that signaled
     // the detach_cond isn't still holding it
     scoped_lock with(detach_lock);

src/rt/rustrt.def.in

@@ -35,7 +35,6 @@ rust_list_files
 rust_log_console_on
 rust_log_console_off
 rust_port_begin_detach
-rust_port_end_detach
 rust_port_size
 rust_process_wait
 rust_ptr_eq

src/rt/sync/sync.h

@@ -29,6 +29,16 @@ class sync {
     static T decrement(T &address) {
         return __sync_sub_and_fetch(&address, 1);
     }
+
+    template <class T>
+    static T read(T *address) {
+        return __sync_add_and_fetch(address, 0);
+    }
+
+    template <class T>
+    static T read(T &address) {
+        return __sync_add_and_fetch(&address, 0);
+    }
 };
 
 #endif /* SYNC_H */