Tidy up the sync dir, remove dead or mis-designed code in favour of OS primitives, switch rust_kernel to use a lock/signal pair and wait rather than spin.

src/Makefile

@@ -248,8 +248,7 @@ BOOT_CMIS := $(BOOT_MLS:.ml=.cmi)
 
 RUNTIME_CS := rt/sync/timer.cpp \
               rt/sync/sync.cpp \
-              rt/sync/spin_lock.cpp \
+              rt/sync/lock_and_signal.cpp \
-              rt/sync/condition_variable.cpp \
               rt/rust.cpp \
               rt/rust_builtin.cpp \
               rt/rust_crate.cpp \
@@ -383,6 +382,9 @@ self: $(CFG_COMPILER)
 TASK_XFAILS := test/run-pass/task-comm-8.rs \
                test/run-pass/task-comm-10.rs \
                test/run-pass/task-comm-15.rs \
+               test/run-pass/task-comm-12.rs \
+               test/run-pass/task-comm-2.rs \
+               test/run-pass/task-comm-9.rs \
                test/run-pass/task-life-0.rs \
                test/run-pass/alt-type-simple.rs \
                test/run-pass/many.rs
@@ -936,3 +938,11 @@ clean:
 	$(CFG_QUIET)rm -Rf $(PKG_NAME)-*.tar.gz dist
 	$(CFG_QUIET)rm -f $(foreach ext,cmx cmi cmo cma o a d exe,\
                         $(wildcard boot/*/*.$(ext) boot/*/*/*.$(ext)))
+
+
+# Local Variables:
+# mode: makefile-gmake
+# fill-column: 78;
+# buffer-file-coding-system: utf-8-unix
+# compile-command: "make -k 2>&1 | sed -e 's/\\/x\\//x:\\//g'";
+# End:

src/rt/memory_region.h

@@ -9,7 +9,7 @@
 #ifndef MEMORY_REGION_H
 #define MEMORY_REGION_H
 
-#include "sync/spin_lock.h"
+#include "sync/lock_and_signal.h"
 
 class rust_srv;
 
@@ -20,7 +20,7 @@ private:
     size_t _live_allocations;
     array_list<void *> _allocation_list;
     const bool _synchronized;
-    spin_lock _lock;
+    lock_and_signal _lock;
 public:
     enum memory_region_type {
         LOCAL = 0x1, SYNCHRONIZED = 0x2
@@ -42,4 +42,15 @@ inline void *operator new(size_t size, memory_region *region) {
     return region->malloc(size);
 }
 
+//
+// Local Variables:
+// mode: C++
+// fill-column: 78;
+// indent-tabs-mode: nil
+// c-basic-offset: 4
+// buffer-file-coding-system: utf-8-unix
+// compile-command: "make -k -C .. 2>&1 | sed -e 's/\\/x\\//x:\\//g'";
+// End:
+//
+
 #endif /* MEMORY_REGION_H */

src/rt/rust_internal.h

@@ -42,7 +42,7 @@ extern "C" {
 #include "util/hash_map.h"
 #include "sync/sync.h"
 #include "sync/timer.h"
-#include "sync/condition_variable.h"
+#include "sync/lock_and_signal.h"
 #include "sync/lock_free_queue.h"
 
 class rust_dom;

src/rt/rust_kernel.cpp

@@ -12,7 +12,7 @@ rust_kernel::rust_kernel(rust_srv *srv) :
 
 rust_handle<rust_dom> *
 rust_kernel::create_domain(const rust_crate *crate, const char *name) {
-    LOCK(_kernel_lock);
+    _kernel_lock.lock();
     rust_message_queue *message_queue =
         new (this) rust_message_queue(_srv, this);
     rust_srv *srv = _srv->clone();
@@ -22,13 +22,14 @@ rust_kernel::create_domain(const rust_crate *crate, const char *name) {
     message_queue->associate(handle);
     domains.append(dom);
     message_queues.append(message_queue);
-    UNLOCK(_kernel_lock);
+    _kernel_lock.unlock();
+    _kernel_lock.signal();
     return handle;
 }
 
 void
 rust_kernel::destroy_domain(rust_dom *dom) {
-    LOCK(_kernel_lock);
+    _kernel_lock.lock();
     log(rust_log::KERN, "deleting domain: " PTR ", index: %d, domains %d",
         dom, dom->list_index, domains.length());
     domains.remove(dom);
@@ -36,7 +37,8 @@ rust_kernel::destroy_domain(rust_dom *dom) {
     rust_srv *srv = dom->srv;
     delete dom;
     delete srv;
-    UNLOCK(_kernel_lock);
+    _kernel_lock.unlock();
+    _kernel_lock.signal();
 }
 
 rust_handle<rust_dom> *
@@ -52,15 +54,15 @@ rust_kernel::internal_get_dom_handle(rust_dom *dom) {
 
 rust_handle<rust_dom> *
 rust_kernel::get_dom_handle(rust_dom *dom) {
-    LOCK(_kernel_lock);
+    _kernel_lock.lock();
     rust_handle<rust_dom> *handle = internal_get_dom_handle(dom);
-    UNLOCK(_kernel_lock);
+    _kernel_lock.unlock();
     return handle;
 }
 
 rust_handle<rust_task> *
 rust_kernel::get_task_handle(rust_task *task) {
-    LOCK(_kernel_lock);
+    _kernel_lock.lock();
     rust_handle<rust_task> *handle = NULL;
     if (_task_handles.get(task, &handle) == false) {
         handle =
@@ -68,13 +70,13 @@ rust_kernel::get_task_handle(rust_task *task) {
                                               task);
         _task_handles.put(task, handle);
     }
-    UNLOCK(_kernel_lock);
+    _kernel_lock.unlock();
     return handle;
 }
 
 rust_handle<rust_port> *
 rust_kernel::get_port_handle(rust_port *port) {
-    PLOCK(_kernel_lock);
+    _kernel_lock.lock();
     rust_handle<rust_port> *handle = NULL;
     if (_port_handles.get(port, &handle) == false) {
         handle =
@@ -83,7 +85,7 @@ rust_kernel::get_port_handle(rust_port *port) {
                                               port);
         _port_handles.put(port, handle);
     }
-    PUNLOCK(_kernel_lock);
+    _kernel_lock.unlock();
     return handle;
 }
 
@@ -126,7 +128,6 @@ rust_kernel::log(uint32_t type_bits, char const *fmt, ...) {
 
 void
 rust_kernel::pump_message_queues() {
-    LOCK(_kernel_lock);
     for (size_t i = 0; i < message_queues.length(); i++) {
         rust_message_queue *queue = message_queues[i];
         if (queue->is_associated() == false) {
@@ -137,27 +138,16 @@ rust_kernel::pump_message_queues() {
             }
         }
     }
-    UNLOCK(_kernel_lock);
 }
 
 void
 rust_kernel::start_kernel_loop() {
+    _kernel_lock.lock();
     while (_interrupt_kernel_loop == false) {
+        _kernel_lock.wait();
         pump_message_queues();
-
-        // FIXME: this is a complete hack to make the testsuite finish in a
-        // sane time when executing under valgrind. The whole message-loop
-        // system here needs replacement with an OS-level event-queue such
-        // that actually wait on inter-thread notices, rather than
-        // busy-waiting.
-
-        size_t ms = TIME_SLICE_IN_MS;
-#if defined(__WIN32__)
-        Sleep(ms);
-#else
-        usleep(ms * 1000);
-#endif
     }
+    _kernel_lock.unlock();
 }
 
 void
@@ -171,6 +161,7 @@ void
 rust_kernel::terminate_kernel_loop() {
     log(rust_log::KERN, "terminating kernel loop");
     _interrupt_kernel_loop = true;
+    _kernel_lock.signal();
     join();
 }
 

src/rt/rust_kernel.h

@@ -32,12 +32,6 @@ public:
     }
 };
 
-#define LOCK(x) x.lock();
-#define UNLOCK(x) x.unlock();
-
-#define PLOCK(x) printf("LOCKING @ %d\n", __LINE__); x.lock();
-#define PUNLOCK(x) x.unlock(); printf("UNLOCKED @ %d\n", __LINE__);
-
 /**
  * A global object shared by all thread domains. Most of the data structures
  * in this class are synchronized since they are accessed from multiple
@@ -59,9 +53,9 @@ class rust_kernel : public rust_thread {
 
     void run();
     void start_kernel_loop();
-    bool volatile _interrupt_kernel_loop;
+    bool _interrupt_kernel_loop;
 
-    spin_lock _kernel_lock;
+    lock_and_signal _kernel_lock;
 
     void terminate_kernel_loop();
     void pump_message_queues();

src/rt/rust_log.cpp

@@ -4,7 +4,6 @@
  */
 
 #include "rust_internal.h"
-#include "sync/spin_lock.h"
 #include "util/array_list.h"
 #include <stdarg.h>
 
@@ -64,7 +63,7 @@ static const char * _foreground_colors[] = { "[37m",
 /**
  * Synchronizes access to the underlying logging mechanism.
  */
-static spin_lock _log_lock;
+static lock_and_signal _log_lock;
 static uint32_t _last_thread_id;
 
 rust_log::rust_log(rust_srv *srv, rust_dom *dom) :

src/rt/sync/condition_variable.cpp

@@ -1,84 +0,0 @@
-#include "../globals.h"
-
-/*
- * Conditional variable. Implemented using pthreads condition variables, and
- * using events on windows.
- */
-
-#include "condition_variable.h"
-
-// #define TRACE
-
-#if defined(__WIN32__)
-condition_variable::condition_variable() {
-    _event = CreateEvent(NULL, FALSE, FALSE, NULL);
-}
-#else
-condition_variable::condition_variable() {
-    pthread_cond_init(&_cond, NULL);
-    pthread_mutex_init(&_mutex, NULL);
-}
-#endif
-
-condition_variable::~condition_variable() {
-#if defined(__WIN32__)
-    CloseHandle(_event);
-#else
-    pthread_cond_destroy(&_cond);
-    pthread_mutex_destroy(&_mutex);
-#endif
-}
-
-/**
- * Wait indefinitely until condition is signaled.
- */
-void condition_variable::wait() {
-    timed_wait(0);
-}
-
-void condition_variable::timed_wait(size_t timeout_in_ns) {
-#ifdef TRACE
-    printf("waiting on condition_variable: 0x%" PRIxPTR " for %d ns. \n",
-           (uintptr_t) this, (int) timeout_in_ns);
-#endif
-#if defined(__WIN32__)
-    WaitForSingleObject(_event, INFINITE);
-#else
-    pthread_mutex_lock(&_mutex);
-    // wait() automatically releases the mutex while it waits, and acquires
-    // it right before exiting. This allows signal() to acquire the mutex
-    // when signaling.)
-    if (timeout_in_ns == 0) {
-        pthread_cond_wait(&_cond, &_mutex);
-    } else {
-        timeval time_val;
-        gettimeofday(&time_val, NULL);
-        timespec time_spec;
-        time_spec.tv_sec = time_val.tv_sec + 0;
-        time_spec.tv_nsec = time_val.tv_usec * 1000 + timeout_in_ns;
-        pthread_cond_timedwait(&_cond, &_mutex, &time_spec);
-    }
-    pthread_mutex_unlock(&_mutex);
-#endif
-#ifdef TRACE
-    printf("resumed on condition_variable: 0x%" PRIxPTR "\n",
-           (uintptr_t)this);
-#endif
-}
-
-/**
- * Signal condition, and resume the waiting thread.
- */
-void condition_variable::signal() {
-#if defined(__WIN32__)
-    SetEvent(_event);
-#else
-    pthread_mutex_lock(&_mutex);
-    pthread_cond_signal(&_cond);
-    pthread_mutex_unlock(&_mutex);
-#endif
-#ifdef TRACE
-    printf("signal  condition_variable: 0x%" PRIxPTR "\n",
-           (uintptr_t)this);
-#endif
-}

src/rt/sync/condition_variable.h

@@ -1,20 +0,0 @@
-#ifndef CONDITION_VARIABLE_H
-#define CONDITION_VARIABLE_H
-
-class condition_variable {
-#if defined(__WIN32__)
-    HANDLE _event;
-#else
-    pthread_cond_t _cond;
-    pthread_mutex_t _mutex;
-#endif
-public:
-    condition_variable();
-    virtual ~condition_variable();
-
-    void wait();
-    void timed_wait(size_t timeout_in_ns);
-    void signal();
-};
-
-#endif /* CONDITION_VARIABLE_H */

src/rt/sync/fair_ticket_lock.cpp

@@ -1,43 +0,0 @@
-/*
- * This works well as long as the number of contending threads
- * is less than the number of processors. This is because of
- * the fair locking scheme. If the thread that is next in line
- * for acquiring the lock is not currently running, no other
- * thread can acquire the lock. This is terrible for performance,
- * and it seems that all fair locking schemes suffer from this
- * behavior.
- */
-
-// #define TRACE
-
-fair_ticket_lock::fair_ticket_lock() {
-    next_ticket = now_serving = 0;
-}
-
-fair_ticket_lock::~fair_ticket_lock() {
-
-}
-
-void fair_ticket_lock::lock() {
-    unsigned ticket = __sync_fetch_and_add(&next_ticket, 1);
-    while (now_serving != ticket) {
-        pause();
-    }
-#ifdef TRACE
-    printf("locked   nextTicket: %d nowServing: %d",
-            next_ticket, now_serving);
-#endif
-}
-
-void fair_ticket_lock::unlock() {
-    now_serving++;
-#ifdef TRACE
-    printf("unlocked nextTicket: %d nowServing: %d",
-            next_ticket, now_serving);
-#endif
-}
-
-void fair_ticket_lock::pause() {
-    asm volatile("pause\n" : : : "memory");
-}
-

src/rt/sync/fair_ticket_lock.h

@@ -1,15 +0,0 @@
-#ifndef FAIR_TICKET_LOCK_H
-#define FAIR_TICKET_LOCK_H
-
-class fair_ticket_lock {
-    unsigned next_ticket;
-    unsigned now_serving;
-    void pause();
-public:
-    fair_ticket_lock();
-    virtual ~fair_ticket_lock();
-    void lock();
-    void unlock();
-};
-
-#endif /* FAIR_TICKET_LOCK_H */

src/rt/sync/interrupt_transparent_queue.cpp

@@ -1,56 +0,0 @@
-/*
- * Interrupt transparent queue, Schoen et. al, "On Interrupt-Transparent
- * Synchronization in an Embedded Object-Oriented Operating System", 2000.
- * enqueue() is allowed to interrupt enqueue() and dequeue(), however,
- * dequeue() is not allowed to interrupt itself.
- */
-
-#include "../globals.h"
-#include "interrupt_transparent_queue.h"
-
-interrupt_transparent_queue_node::interrupt_transparent_queue_node() :
-    next(NULL) {
-
-}
-
-interrupt_transparent_queue::interrupt_transparent_queue() : _tail(this) {
-
-}
-
-void
-interrupt_transparent_queue::enqueue(interrupt_transparent_queue_node *item) {
-    lock.lock();
-    item->next = (interrupt_transparent_queue_node *) NULL;
-    interrupt_transparent_queue_node *last = _tail;
-    _tail = item;
-    while (last->next) {
-        last = last->next;
-    }
-    last->next = item;
-    lock.unlock();
-}
-
-interrupt_transparent_queue_node *
-interrupt_transparent_queue::dequeue() {
-    lock.lock();
-    interrupt_transparent_queue_node *item = next;
-    if (item && !(next = item->next)) {
-        _tail = (interrupt_transparent_queue_node *) this;
-        if (item->next) {
-            interrupt_transparent_queue_node *lost = item->next;
-            interrupt_transparent_queue_node *help;
-            do {
-                help = lost->next;
-                enqueue(lost);
-            } while ((lost = help) !=
-                     (interrupt_transparent_queue_node *) NULL);
-        }
-    }
-    lock.unlock();
-    return item;
-}
-
-bool
-interrupt_transparent_queue::is_empty() {
-    return next == NULL;
-}

src/rt/sync/interrupt_transparent_queue.h

@@ -1,22 +0,0 @@
-#ifndef INTERRUPT_TRANSPARENT_QUEUE_H
-#define INTERRUPT_TRANSPARENT_QUEUE_H
-
-#include "spin_lock.h"
-
-class interrupt_transparent_queue_node {
-public:
-    interrupt_transparent_queue_node *next;
-    interrupt_transparent_queue_node();
-};
-
-class interrupt_transparent_queue : interrupt_transparent_queue_node {
-    spin_lock lock;
-    interrupt_transparent_queue_node *_tail;
-public:
-    interrupt_transparent_queue();
-    void enqueue(interrupt_transparent_queue_node *item);
-    interrupt_transparent_queue_node *dequeue();
-    bool is_empty();
-};
-
-#endif /* INTERRUPT_TRANSPARENT_QUEUE_H */

src/rt/sync/lock_and_signal.cpp

@@ -0,0 +1,97 @@
+#include "../globals.h"
+
+/*
+ * A "lock-and-signal" pair. These are necessarily coupled on pthreads
+ * systems, and artificially coupled (by this file) on win32. Put
+ * together here to minimize ifdefs elsewhere; you must use them as
+ * if you're using a pthreads cvar+mutex pair.
+ */
+
+#include "lock_and_signal.h"
+
+#if defined(__WIN32__)
+lock_and_signal::lock_and_signal() {
+    _event = CreateEvent(NULL, FALSE, FALSE, NULL);
+    InitializeCriticalSection(&_cs);
+}
+
+#else
+lock_and_signal::lock_and_signal() {
+    pthread_cond_init(&_cond, NULL);
+    pthread_mutex_init(&_mutex, NULL);
+}
+#endif
+
+lock_and_signal::~lock_and_signal() {
+#if defined(__WIN32__)
+    CloseHandle(_event);
+#else
+    pthread_cond_destroy(&_cond);
+    pthread_mutex_destroy(&_mutex);
+#endif
+}
+
+void lock_and_signal::lock() {
+#if defined(__WIN32__)
+  EnterCriticalSection(&_cs);
+#else
+  pthread_mutex_lock(&_mutex);
+#endif
+}
+
+void lock_and_signal::unlock() {
+#if defined(__WIN32__)
+  LeaveCriticalSection(&_cs);
+#else
+  pthread_mutex_unlock(&_mutex);
+#endif
+}
+
+
+/**
+ * Wait indefinitely until condition is signaled.
+ */
+void lock_and_signal::wait() {
+    timed_wait(0);
+}
+
+void lock_and_signal::timed_wait(size_t timeout_in_ns) {
+#if defined(__WIN32__)
+  LeaveCriticalSection(&_cs);
+  WaitForSingleObject(_event, INFINITE);
+  EnterCriticalSection(&_cs);
+#else
+    if (timeout_in_ns == 0) {
+        pthread_cond_wait(&_cond, &_mutex);
+    } else {
+        timeval time_val;
+        gettimeofday(&time_val, NULL);
+        timespec time_spec;
+        time_spec.tv_sec = time_val.tv_sec + 0;
+        time_spec.tv_nsec = time_val.tv_usec * 1000 + timeout_in_ns;
+        pthread_cond_timedwait(&_cond, &_mutex, &time_spec);
+    }
+#endif
+}
+
+/**
+ * Signal condition, and resume the waiting thread.
+ */
+void lock_and_signal::signal() {
+#if defined(__WIN32__)
+    SetEvent(_event);
+#else
+    pthread_cond_signal(&_cond);
+#endif
+}
+
+
+//
+// Local Variables:
+// mode: C++
+// fill-column: 78;
+// indent-tabs-mode: nil
+// c-basic-offset: 4
+// buffer-file-coding-system: utf-8-unix
+// compile-command: "make -k -C .. 2>&1 | sed -e 's/\\/x\\//x:\\//g'";
+// End:

src/rt/sync/lock_and_signal.h

@@ -0,0 +1,23 @@
+#ifndef LOCK_AND_SIGNAL_H
+#define LOCK_AND_SIGNAL_H
+
+class lock_and_signal {
+#if defined(__WIN32__)
+    HANDLE _event;
+    CRITICAL_SECTION _cs;
+#else
+    pthread_cond_t _cond;
+    pthread_mutex_t _mutex;
+#endif
+public:
+    lock_and_signal();
+    virtual ~lock_and_signal();
+
+    void lock();
+    void unlock();
+    void wait();
+    void timed_wait(size_t timeout_in_ns);
+    void signal();
+};
+
+#endif /* LOCK_AND_SIGNAL_H */

src/rt/sync/spin_lock.cpp

@@ -1,48 +0,0 @@
-#include "../globals.h"
-#include "spin_lock.h"
-
-/*
- * Your average spin lock.
- */
-
-// #define TRACE
-
-spin_lock::spin_lock() {
-    unlock();
-}
-
-spin_lock::~spin_lock() {
-}
-
-static inline unsigned xchg32(void *ptr, unsigned x) {
-    __asm__ __volatile__("xchgl %0,%1"
-                :"=r" ((unsigned) x)
-                :"m" (*(volatile unsigned *)ptr), "0" (x)
-                :"memory");
-    return x;
-}
-
-void spin_lock::lock() {
-    while (true) {
-        if (!xchg32(&ticket, 1)) {
-            return;
-        }
-        while (ticket) {
-            pause();
-        }
-    }
-#ifdef TRACE
-    printf("  lock: %d", ticket);
-#endif
-}
-
-void spin_lock::unlock() {
-    ticket = 0;
-#ifdef TRACE
-    printf("unlock:");
-#endif
-}
-
-void spin_lock::pause() {
-    asm volatile("pause\n" : : : "memory");
-}

src/rt/sync/spin_lock.h

@@ -1,14 +0,0 @@
-#ifndef SPIN_LOCK_H
-#define SPIN_LOCK_H
-
-class spin_lock {
-    unsigned ticket;
-    void pause();
-public:
-    spin_lock();
-    virtual ~spin_lock();
-    void lock();
-    void unlock();
-};
-
-#endif /* SPIN_LOCK_H */

src/rt/sync/sync.cpp

@@ -11,7 +11,7 @@ void sync::yield() {
 #endif
 }
 
-rust_thread::rust_thread() : _is_running(false) {
+rust_thread::rust_thread() : _is_running(false), thread(0) {
     // Nop.
 }
 
@@ -25,7 +25,6 @@ static void *
 rust_thread_start(void *ptr) {
     rust_thread *thread = (rust_thread *) ptr;
     thread->run();
-    thread->thread = 0;
     return 0;
 }
 
@@ -46,8 +45,10 @@ rust_thread::start() {
 void
 rust_thread::join() {
 #if defined(__WIN32__)
+   if (thread)
      WaitForSingleObject(thread, INFINITE);
 #else
+   if (thread)
      pthread_join(thread, NULL);
 #endif
    thread = 0;

src/rt/util/synchronized_indexed_list.h

@@ -5,7 +5,7 @@
 
 template<typename T> class synchronized_indexed_list :
     public indexed_list<T> {
-    spin_lock _lock;
+    lock_and_signal _lock;
 
 public:
     synchronized_indexed_list(memory_region *region) :
@@ -61,4 +61,15 @@ public:
     }
 };
 
+//
+// Local Variables:
+// mode: C++
+// fill-column: 78;
+// indent-tabs-mode: nil
+// c-basic-offset: 4
+// buffer-file-coding-system: utf-8-unix
+// compile-command: "make -k -C ../.. 2>&1 | sed -e 's/\\/x\\//x:\\//g'";
+// End:
+//
+
 #endif /* SYNCHRONIZED_INDEXED_LIST_H */