Merge branch 'cond-n-timedlock' into 'master'

Release pthread cond workaround, fix mutex timedlock

See merge request redox-os/relibc!870

src/header/pthread/mutex.rs

@@ -1,6 +1,12 @@
 use super::*;
 
-use crate::error::Errno;
+use crate::{
+    error::Errno,
+    header::{
+        errno::{EINVAL, ENOMEM, ETIMEDOUT},
+        time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec_realtime_to_monotonic},
+    },
+};
 
 // PTHREAD_MUTEX_INITIALIZER is defined in bits_pthread/cbindgen.toml
 
@@ -72,10 +78,16 @@ pub unsafe extern "C" fn pthread_mutex_setprioceiling(
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_mutex_timedlock(
     mutex: *mut pthread_mutex_t,
-    timespec: *const timespec,
+    abstime: *const timespec,
 ) -> c_int {
-    e((&*mutex.cast::<RlctMutex>()).lock_with_timeout(&*timespec))
+    let relative = match timespec_realtime_to_monotonic(*abstime) {
+        Ok(relative) => relative,
+        Err(err) => return e(Err(err)),
+    };
+
+    e((&*mutex.cast::<RlctMutex>()).lock_with_timeout(&relative))
 }
+
 #[unsafe(no_mangle)]
 pub unsafe extern "C" fn pthread_mutex_trylock(mutex: *mut pthread_mutex_t) -> c_int {
     e((&*mutex.cast::<RlctMutex>()).try_lock())

src/header/time/mod.rs

@@ -7,7 +7,7 @@ use crate::{
     error::{Errno, ResultExt},
     fs::File,
     header::{
-        errno::{EFAULT, EOVERFLOW},
+        errno::{EFAULT, ENOMEM, EOVERFLOW, ETIMEDOUT},
         fcntl::O_RDONLY,
         signal::sigevent,
         stdlib::getenv,
@@ -848,3 +848,17 @@ const fn blank_tm() -> tm {
         tm_zone: ptr::null_mut(),
     }
 }
+
+pub(crate) fn timespec_realtime_to_monotonic(abstime: timespec) -> Result<timespec, Errno> {
+    let mut realtime = timespec::default();
+    unsafe { clock_gettime(CLOCK_REALTIME, &mut realtime) };
+    let mut monotonic = timespec::default();
+    unsafe { clock_gettime(CLOCK_MONOTONIC, &mut monotonic) };
+    let Some(delta) = timespec::subtract(abstime, realtime) else {
+        return Err(Errno(ETIMEDOUT));
+    };
+    let Some(relative) = timespec::add(monotonic, delta) else {
+        return Err(Errno(ENOMEM));
+    };
+    Ok(relative)
+}

src/sync/cond.rs

@@ -5,7 +5,7 @@ use crate::{
     header::{
         errno::{EINVAL, ENOMEM, ETIMEDOUT},
         pthread::*,
-        time::{CLOCK_MONOTONIC, CLOCK_REALTIME, clock_gettime, timespec},
+        time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec, timespec_realtime_to_monotonic},
     },
     platform::types::clockid_t,
 };
@@ -69,53 +69,24 @@ impl Cond {
         timeout: &timespec,
         clock_id: clockid_t,
     ) -> Result<(), Errno> {
-        // adjusted timeout similar in semaphore.rs
-
         let relative = match clock_id {
             // FUTEX expect monotonic clock
             CLOCK_MONOTONIC => timeout.clone(),
-            CLOCK_REALTIME => {
-                let mut realtime = timespec::default();
-                unsafe { clock_gettime(CLOCK_REALTIME, &mut realtime) };
-                let mut monotonic = timespec::default();
-                unsafe { clock_gettime(CLOCK_MONOTONIC, &mut monotonic) };
-                let Some(delta) = timespec::subtract(timeout.clone(), realtime) else {
-                    return Err(Errno(ETIMEDOUT));
-                };
-                let Some(relative) = timespec::add(monotonic, delta) else {
-                    return Err(Errno(ENOMEM));
-                };
-                relative
-            }
+            CLOCK_REALTIME => match timespec_realtime_to_monotonic(timeout.clone()) {
+                Ok(relative) => relative,
+                Err(err) => return Err(err),
+            },
             _ => return Err(Errno(EINVAL)),
         };
 
-        match self.wait_inner(mutex, Some(&relative)) {
-            Err(Errno(ETIMEDOUT)) => {
-                // TODO: this is a workaround that SYS_FUTEX returns ETIMEDOUT but actually it signaled in time
-                let mut monotonic = timespec::default();
-                unsafe { clock_gettime(CLOCK_MONOTONIC, &mut monotonic) };
-
-                if timespec::subtract(relative, monotonic).is_some() {
-                    Ok(())
-                } else {
-                    Err(Errno(ETIMEDOUT))
-                }
-            }
-            r => r,
-        }
+        self.wait_inner(mutex, Some(&relative))
     }
     pub fn timedwait(&self, mutex: &RlctMutex, timeout: &timespec) -> Result<(), Errno> {
         // TODO: The clock can be other than CLOCK_REALTIME depends on CondAttr
         self.clockwait(mutex, timeout, CLOCK_REALTIME)
     }
     fn wait_inner(&self, mutex: &RlctMutex, timeout: Option<&timespec>) -> Result<(), Errno> {
-        self.wait_inner_generic(
-            || mutex.unlock(),
-            || mutex.lock(),
-            |timeout| mutex.lock_with_timeout(timeout),
-            timeout,
-        )
+        self.wait_inner_generic(|| mutex.unlock(), || mutex.lock(), timeout)
     }
     pub fn wait_inner_typedmutex<'lock, T>(
         &self,
@@ -132,7 +103,6 @@ impl Cond {
                 newguard = Some(lock.lock());
                 Ok(())
             },
-            |_| unreachable!(),
             None,
         )
         .unwrap();
@@ -143,7 +113,6 @@ impl Cond {
         &self,
         unlock: impl FnOnce() -> Result<()>,
         lock: impl FnOnce() -> Result<()>,
-        lock_with_timeout: impl FnOnce(&timespec) -> Result<()>,
         deadline: Option<&timespec>,
     ) -> Result<(), Errno> {
         // TODO: Error checking for certain types (i.e. robust and errorcheck) of mutexes, e.g. if the
@@ -151,20 +120,9 @@ impl Cond {
         let current = self.cur.load(Ordering::Relaxed);
         self.prev.store(current, Ordering::Relaxed);
 
-        unlock();
-
-        let futex_r = match deadline {
-            Some(deadline) => {
-                let r = crate::sync::futex_wait(&self.cur, current, Some(&deadline));
-                lock_with_timeout(deadline)?;
-                r
-            }
-            None => {
-                let r = crate::sync::futex_wait(&self.cur, current, None);
-                lock()?;
-                r
-            }
-        };
+        unlock()?;
+        let futex_r = crate::sync::futex_wait(&self.cur, current, deadline);
+        lock()?;
 
         match futex_r {
             super::FutexWaitResult::Waited => Ok(()),

src/sync/semaphore.rs

@@ -2,7 +2,7 @@
 //TODO: improve implementation
 
 use crate::{
-    header::time::{CLOCK_MONOTONIC, CLOCK_REALTIME, clock_gettime, timespec},
+    header::time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec, timespec_realtime_to_monotonic},
     platform::types::{c_uint, clockid_t},
 };
 
@@ -63,27 +63,12 @@ impl Semaphore {
                 let relative = match clock_id {
                     // FUTEX expect monotonic clock
                     CLOCK_MONOTONIC => timeout.clone(),
-                    CLOCK_REALTIME => {
-                        let mut realtime = timespec::default();
-                        unsafe { clock_gettime(CLOCK_REALTIME, &mut realtime) };
-
-                        let mut monotonic = timespec::default();
-                        unsafe { clock_gettime(CLOCK_MONOTONIC, &mut monotonic) };
-
-                        let Some(delta) = timespec::subtract(timeout.clone(), realtime) else {
-                            //Timeout happened
-                            return Err(());
-                        };
-
-                        let Some(relative) = timespec::add(monotonic, delta) else {
-                            return Err(());
-                        };
-
-                        relative
-                    }
+                    CLOCK_REALTIME => match timespec_realtime_to_monotonic(timeout.clone()) {
+                        Ok(relative) => relative,
+                        Err(_) => return Err(()),
+                    },
                     _ => return Err(()),
                 };
-
                 crate::sync::futex_wait(&self.count, value, Some(&relative));
             } else {
                 // Use futex to wait for the next change, without a timeout