Rollup merge of rust-lang#72954 - hermitcore:rwlock, r=dtolnay

revise RwLock for HermitCore

- current version is derived from the wasm implementation
- increasing the readability of `Condvar`
- simplify the interface to the libos

Author: Dylan-DPC

Cargo.lock

@@ -1242,9 +1242,9 @@ dependencies = [
 
 [[package]]
 name = "hermit-abi"
-version = "0.1.14"
+version = "0.1.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "b9586eedd4ce6b3c498bc3b4dd92fc9f11166aa908a914071953768066c67909"
+checksum = "3deed196b6e7f9e44a2ae8d94225d80302d81208b1bb673fd21fe634645c85a9"
 dependencies = [
  "compiler_builtins",
  "libc",

src/libstd/Cargo.toml

@@ -40,7 +40,7 @@ dlmalloc = { version = "0.1", features = ['rustc-dep-of-std'] }
 fortanix-sgx-abi = { version = "0.3.2", features = ['rustc-dep-of-std'] }
 
 [target.'cfg(all(any(target_arch = "x86_64", target_arch = "aarch64"), target_os = "hermit"))'.dependencies]
-hermit-abi = { version = "0.1.14", features = ['rustc-dep-of-std'] }
+hermit-abi = { version = "0.1.15", features = ['rustc-dep-of-std'] }
 
 [target.wasm32-wasi.dependencies]
 wasi = { version = "0.9.0", features = ['rustc-dep-of-std'], default-features = false }

src/libstd/sys/hermit/condvar.rs

@@ -1,60 +1,64 @@
-use crate::cmp;
+use crate::ffi::c_void;
+use crate::ptr;
+use crate::sync::atomic::{AtomicUsize, Ordering::SeqCst};
 use crate::sys::hermit::abi;
 use crate::sys::mutex::Mutex;
 use crate::time::Duration;
 
+// The implementation is inspired by Andrew D. Birrell's paper
+// "Implementing Condition Variables with Semaphores"
+
 pub struct Condvar {
-    identifier: usize,
+    counter: AtomicUsize,
+    sem1: *const c_void,
+    sem2: *const c_void,
 }
 
+unsafe impl Send for Condvar {}
+unsafe impl Sync for Condvar {}
+
 impl Condvar {
     pub const fn new() -> Condvar {
-        Condvar { identifier: 0 }
+        Condvar { counter: AtomicUsize::new(0), sem1: ptr::null(), sem2: ptr::null() }
     }
 
     pub unsafe fn init(&mut self) {
-        let _ = abi::init_queue(self.id());
+        let _ = abi::sem_init(&mut self.sem1 as *mut *const c_void, 0);
+        let _ = abi::sem_init(&mut self.sem2 as *mut *const c_void, 0);
     }
 
     pub unsafe fn notify_one(&self) {
-        let _ = abi::notify(self.id(), 1);
+        if self.counter.load(SeqCst) > 0 {
+            self.counter.fetch_sub(1, SeqCst);
+            abi::sem_post(self.sem1);
+            abi::sem_timedwait(self.sem2, 0);
+        }
     }
 
-    #[inline]
     pub unsafe fn notify_all(&self) {
-        let _ = abi::notify(self.id(), -1 /* =all */);
+        let counter = self.counter.swap(0, SeqCst);
+        for _ in 0..counter {
+            abi::sem_post(self.sem1);
+        }
+        for _ in 0..counter {
+            abi::sem_timedwait(self.sem2, 0);
+        }
     }
 
     pub unsafe fn wait(&self, mutex: &Mutex) {
-        // add current task to the wait queue
-        let _ = abi::add_queue(self.id(), -1 /* no timeout */);
+        self.counter.fetch_add(1, SeqCst);
         mutex.unlock();
-        let _ = abi::wait(self.id());
+        abi::sem_timedwait(self.sem1, 0);
+        abi::sem_post(self.sem2);
         mutex.lock();
     }
 
-    pub unsafe fn wait_timeout(&self, mutex: &Mutex, dur: Duration) -> bool {
-        let nanos = dur.as_nanos();
-        let nanos = cmp::min(i64::MAX as u128, nanos);
-
-        // add current task to the wait queue
-        let _ = abi::add_queue(self.id(), nanos as i64);
-
-        mutex.unlock();
-        // If the return value is !0 then a timeout happened, so we return
-        // `false` as we weren't actually notified.
-        let ret = abi::wait(self.id()) == 0;
-        mutex.lock();
-
-        ret
+    pub unsafe fn wait_timeout(&self, _mutex: &Mutex, _dur: Duration) -> bool {
+        panic!("wait_timeout not supported on hermit");
     }
 
     pub unsafe fn destroy(&self) {
-        let _ = abi::destroy_queue(self.id());
-    }
-
-    #[inline]
-    fn id(&self) -> usize {
-        &self.identifier as *const usize as usize
+        let _ = abi::sem_destroy(self.sem1);
+        let _ = abi::sem_destroy(self.sem2);
     }
 }

src/libstd/sys/hermit/rwlock.rs

@@ -1,49 +1,144 @@
-use super::mutex::Mutex;
+use crate::cell::UnsafeCell;
+use crate::sys::condvar::Condvar;
+use crate::sys::mutex::Mutex;
 
 pub struct RWLock {
-    mutex: Mutex,
+    lock: Mutex,
+    cond: Condvar,
+    state: UnsafeCell<State>,
+}
+
+enum State {
+    Unlocked,
+    Reading(usize),
+    Writing,
 }
 
 unsafe impl Send for RWLock {}
 unsafe impl Sync for RWLock {}
 
+// This rwlock implementation is a relatively simple implementation which has a
+// condition variable for readers/writers as well as a mutex protecting the
+// internal state of the lock. A current downside of the implementation is that
+// unlocking the lock will notify *all* waiters rather than just readers or just
+// writers. This can cause lots of "thundering stampede" problems. While
+// hopefully correct this implementation is very likely to want to be changed in
+// the future.
+
 impl RWLock {
     pub const fn new() -> RWLock {
-        RWLock { mutex: Mutex::new() }
+        RWLock { lock: Mutex::new(), cond: Condvar::new(), state: UnsafeCell::new(State::Unlocked) }
     }
 
     #[inline]
     pub unsafe fn read(&self) {
-        self.mutex.lock();
+        self.lock.lock();
+        while !(*self.state.get()).inc_readers() {
+            self.cond.wait(&self.lock);
+        }
+        self.lock.unlock();
     }
 
     #[inline]
     pub unsafe fn try_read(&self) -> bool {
-        self.mutex.try_lock()
+        self.lock.lock();
+        let ok = (*self.state.get()).inc_readers();
+        self.lock.unlock();
+        return ok;
     }
 
     #[inline]
     pub unsafe fn write(&self) {
-        self.mutex.lock();
+        self.lock.lock();
+        while !(*self.state.get()).inc_writers() {
+            self.cond.wait(&self.lock);
+        }
+        self.lock.unlock();
     }
 
     #[inline]
     pub unsafe fn try_write(&self) -> bool {
-        self.mutex.try_lock()
+        self.lock.lock();
+        let ok = (*self.state.get()).inc_writers();
+        self.lock.unlock();
+        return ok;
     }
 
     #[inline]
     pub unsafe fn read_unlock(&self) {
-        self.mutex.unlock();
+        self.lock.lock();
+        let notify = (*self.state.get()).dec_readers();
+        self.lock.unlock();
+        if notify {
+            // FIXME: should only wake up one of these some of the time
+            self.cond.notify_all();
+        }
     }
 
     #[inline]
     pub unsafe fn write_unlock(&self) {
-        self.mutex.unlock();
+        self.lock.lock();
+        (*self.state.get()).dec_writers();
+        self.lock.unlock();
+        // FIXME: should only wake up one of these some of the time
+        self.cond.notify_all();
     }
 
     #[inline]
     pub unsafe fn destroy(&self) {
-        self.mutex.destroy();
+        self.lock.destroy();
+        self.cond.destroy();
+    }
+}
+
+impl State {
+    fn inc_readers(&mut self) -> bool {
+        match *self {
+            State::Unlocked => {
+                *self = State::Reading(1);
+                true
+            }
+            State::Reading(ref mut cnt) => {
+                *cnt += 1;
+                true
+            }
+            State::Writing => false,
+        }
+    }
+
+    fn inc_writers(&mut self) -> bool {
+        match *self {
+            State::Unlocked => {
+                *self = State::Writing;
+                true
+            }
+            State::Reading(_) | State::Writing => false,
+        }
+    }
+
+    fn dec_readers(&mut self) -> bool {
+        let zero = match *self {
+            State::Reading(ref mut cnt) => {
+                *cnt -= 1;
+                *cnt == 0
+            }
+            State::Unlocked | State::Writing => invalid(),
+        };
+        if zero {
+            *self = State::Unlocked;
+        }
+        zero
     }
+
+    fn dec_writers(&mut self) {
+        match *self {
+            State::Writing => {}
+            State::Unlocked | State::Reading(_) => invalid(),
+        }
+        *self = State::Unlocked;
+    }
+}
+
+fn invalid() -> ! {
+    panic!("inconsistent rwlock");
 }