Merge branch 'rwlock-timeout' into 'master'

Implement clockid_t handling to pthread_rwlock timeout

See merge request redox-os/relibc!1139

src/header/pthread/rwlock.rs

@@ -2,7 +2,7 @@ use super::*;
 
 use crate::header::errno::{EBUSY, EINVAL};
 
-use crate::pthread::Pshared;
+use crate::{header::time::CLOCK_REALTIME, pthread::Pshared};
 
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_rwlock_init(
@@ -23,27 +23,52 @@ pub unsafe extern "C" fn pthread_rwlock_init(
 }
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_rwlock_rdlock(rwlock: *mut pthread_rwlock_t) -> c_int {
-    unsafe { get(rwlock) }.acquire_read_lock(None);
-
-    0
+    match unsafe { get(rwlock) }.acquire_read_lock(None) {
+        Ok(()) => 0,
+        Err(e) => e.0,
+    }
+}
+#[unsafe(no_mangle)]
+pub unsafe extern "C" fn pthread_rwlock_clockrdlock(
+    rwlock: *mut pthread_rwlock_t,
+    clock_id: clockid_t,
+    abstime: *const timespec,
+) -> c_int {
+    match unsafe { get(rwlock) }.acquire_read_lock(Some((unsafe { &*abstime }, clock_id))) {
+        Ok(()) => 0,
+        Err(e) => e.0,
+    }
 }
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_rwlock_timedrdlock(
     rwlock: *mut pthread_rwlock_t,
-    timeout: *const timespec,
+    abstime: *const timespec,
 ) -> c_int {
-    unsafe { get(rwlock) }.acquire_read_lock(Some(unsafe { &*timeout }));
-
-    0
+    match unsafe { get(rwlock) }.acquire_read_lock(Some((unsafe { &*abstime }, CLOCK_REALTIME))) {
+        Ok(()) => 0,
+        Err(e) => e.0,
+    }
+}
+#[unsafe(no_mangle)]
+pub unsafe extern "C" fn pthread_rwlock_clockwrlock(
+    rwlock: *mut pthread_rwlock_t,
+    clock_id: clockid_t,
+    abstime: *const timespec,
+) -> c_int {
+    match unsafe { get(rwlock) }.acquire_write_lock(Some((unsafe { &*abstime }, clock_id))) {
+        Ok(()) => 0,
+        Err(e) => e.0,
+    }
 }
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_rwlock_timedwrlock(
     rwlock: *mut pthread_rwlock_t,
-    timeout: *const timespec,
+    abstime: *const timespec,
 ) -> c_int {
-    unsafe { get(rwlock) }.acquire_write_lock(Some(unsafe { &*timeout }));
-
-    0
+    match unsafe { get(rwlock) }.acquire_write_lock(Some((unsafe { &*abstime }, CLOCK_REALTIME))) {
+        Ok(()) => 0,
+        Err(e) => e.0,
+    }
 }
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_rwlock_tryrdlock(rwlock: *mut pthread_rwlock_t) -> c_int {
@@ -67,9 +92,10 @@ pub unsafe extern "C" fn pthread_rwlock_unlock(rwlock: *mut pthread_rwlock_t) ->
 }
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_rwlock_wrlock(rwlock: *mut pthread_rwlock_t) -> c_int {
-    unsafe { get(rwlock) }.acquire_write_lock(None);
-
-    0
+    match unsafe { get(rwlock) }.acquire_write_lock(None) {
+        Ok(()) => 0,
+        Err(e) => e.0,
+    }
 }
 
 #[unsafe(no_mangle)]

src/sync/rwlock.rs

@@ -4,7 +4,16 @@ use core::{
     sync::atomic::{AtomicU32, Ordering},
 };
 
-use crate::{header::bits_time::timespec, pthread::Pshared};
+use crate::{
+    error::{Errno, Result},
+    header::{
+        bits_time::timespec,
+        errno::{EINVAL, ETIMEDOUT},
+        time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec_realtime_to_monotonic},
+    },
+    platform::types::clockid_t,
+    pthread::Pshared,
+};
 
 pub struct InnerRwLock {
     state: AtomicU32,
@@ -25,7 +34,25 @@ impl InnerRwLock {
             state: AtomicU32::new(0),
         }
     }
-    pub fn acquire_write_lock(&self, deadline: Option<&timespec>) {
+    fn translate_timeout(deadline: Option<(&timespec, i32)>) -> Result<Option<timespec>, Errno> {
+        let relative = match deadline {
+            // FUTEX expect monotonic clock
+            Some((abstime, CLOCK_MONOTONIC)) => Some(abstime.clone()),
+            Some((abstime, CLOCK_REALTIME)) => {
+                Some(timespec_realtime_to_monotonic(abstime.clone())?)
+            }
+            None => None,
+            _ => {
+                return Err(Errno(EINVAL));
+            }
+        };
+        Ok(relative)
+    }
+    pub fn acquire_write_lock(
+        &self,
+        deadline: Option<(&timespec, clockid_t)>,
+    ) -> Result<(), Errno> {
+        let relative = Self::translate_timeout(deadline)?;
         let mut waiting_wr = self.state.load(Ordering::Relaxed) & WAITING_WR;
 
         loop {
@@ -35,7 +62,7 @@ impl InnerRwLock {
                 Ordering::Acquire,
                 Ordering::Relaxed,
             ) {
-                Ok(_) => return,
+                Ok(_) => break,
                 Err(actual) => {
                     let expected = actual;
                     let expected = if actual & COUNT_MASK != EXCLUSIVE {
@@ -50,7 +77,10 @@ impl InnerRwLock {
                     waiting_wr = expected & WAITING_WR;
 
                     if actual & COUNT_MASK > 0 {
-                        let _ = crate::sync::futex_wait(&self.state, expected, deadline);
+                        match crate::sync::futex_wait(&self.state, expected, relative.as_ref()) {
+                            super::FutexWaitResult::TimedOut => return Err(Errno(ETIMEDOUT)),
+                            _ => {}
+                        }
                     } else {
                         // We must avoid blocking indefinitely in our `futex_wait()`, in this case
                         // where it's possible that `self.state == expected` but our futex might
@@ -61,12 +91,19 @@ impl InnerRwLock {
                 }
             }
         }
+
+        Ok(())
     }
-    pub fn acquire_read_lock(&self, deadline: Option<&timespec>) {
-        // TODO: timeout
+    pub fn acquire_read_lock(&self, deadline: Option<(&timespec, clockid_t)>) -> Result<(), Errno> {
+        let relative = Self::translate_timeout(deadline)?;
         while let Err(old) = self.try_acquire_read_lock() {
-            crate::sync::futex_wait(&self.state, old, deadline);
+            match crate::sync::futex_wait(&self.state, old, relative.as_ref()) {
+                super::FutexWaitResult::TimedOut => return Err(Errno(ETIMEDOUT)),
+                _ => {}
+            }
         }
+
+        Ok(())
     }
     pub fn try_acquire_read_lock(&self) -> Result<(), u32> {
         let mut cached = self.state.load(Ordering::Acquire);
@@ -167,12 +204,12 @@ impl<T> RwLock<T> {
 
 impl<T: ?Sized> RwLock<T> {
     pub fn read(&self) -> ReadGuard<'_, T> {
-        self.inner.acquire_read_lock(None);
+        let _ = self.inner.acquire_read_lock(None);
         unsafe { ReadGuard::new(self) }
     }
 
     pub fn write(&self) -> WriteGuard<'_, T> {
-        self.inner.acquire_write_lock(None);
+        let _ = self.inner.acquire_write_lock(None);
         unsafe { WriteGuard::new(self) }
     }
 

tests/pthread/rwlock_trylock.c

@@ -4,8 +4,9 @@
 #include <assert.h>
 #include <pthread.h>
 #include <errno.h>
+#include <time.h>
 
-// Same test logic as test_rwlock_try_write in rustc/library/std/sync/rwlock/tests.rs
+// Adapted from test_rwlock_try_write in rustc/library/std/sync/rwlock/tests.rs
 
 int main(void) {
   int status;
@@ -31,11 +32,25 @@ int main(void) {
   ERROR_IF(pthread_rwlock_rdlock, status, != 0);
 
   status = pthread_rwlock_trywrlock(&rwlock);
-  assert(status == EBUSY);
+  UNEXP_IF(pthread_rwlock_trywrlock, status, != EBUSY);
+
+  struct timespec ts;
+  clock_gettime(CLOCK_REALTIME, &ts);
+  ts.tv_nsec += 200 * 1000000;
+  status = pthread_rwlock_timedwrlock(&rwlock, &ts);
+  UNEXP_IF(pthread_rwlock_timedwrlock, status, != ETIMEDOUT);
 
   status = pthread_rwlock_unlock(&rwlock);
   ERROR_IF(pthread_rwlock_unlock, status, != 0);
 
+  status = pthread_rwlock_trywrlock(&rwlock);
+  ERROR_IF(pthread_rwlock_rdlock, status, != 0);
+
+  clock_gettime(CLOCK_REALTIME, &ts);
+  ts.tv_nsec += 200 * 1000000;
+  status = pthread_rwlock_timedrdlock(&rwlock, &ts);
+  UNEXP_IF(pthread_rwlock_timedrdlock, status, != ETIMEDOUT);
+
   status = pthread_rwlock_destroy(&rwlock);
   ERROR_IF(pthread_rwlock_destroy, status, != 0);