rt: Fix the atomic get_ref_count method to avoid races

5 files changed, 12 insertions, 18 deletions

diff --git a/src/rt/rust_builtin.cpp b/src/rt/rust_builtin.cpp
index 14b18a99252..14b6d0f1962 100644
--- a/src/rt/rust_builtin.cpp
+++ b/src/rt/rust_builtin.cpp
@@ -489,15 +489,11 @@ rust_port_begin_detach(rust_port *port, uintptr_t *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;
 }
 
diff --git a/src/rt/rust_internal.h b/src/rt/rust_internal.h
index 5996c6391ad..7d19fbfa04b 100644
--- a/src/rt/rust_internal.h
+++ b/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);
diff --git a/src/rt/rust_port.cpp b/src/rt/rust_port.cpp
index 3aaa107916d..7ac4cef0e8f 100644
--- a/src/rt/rust_port.cpp
+++ b/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);
diff --git a/src/rt/rustrt.def.in b/src/rt/rustrt.def.in
index 8a170ede233..5de20773326 100644
--- a/src/rt/rustrt.def.in
+++ b/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
diff --git a/src/rt/sync/sync.h b/src/rt/sync/sync.h
index e7d7496a56a..66682f11e9c 100644
--- a/src/rt/sync/sync.h
+++ b/src/rt/sync/sync.h
@@ -29,6 +29,16 @@ public:
     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 */