From 85e6e1e675c30bc93834afcf765b3b71acce884a Mon Sep 17 00:00:00 2001 From: Wildan M Date: Fri, 5 Jun 2026 09:21:50 +0700 Subject: [PATCH] Fix sem_trywait and sem_wait error handling --- src/header/semaphore/mod.rs | 42 ++++++++++++++++++---------- src/sync/semaphore.rs | 37 +++++++++++++----------- tests/Makefile.tests.mk | 1 + tests/expected/semaphore/lock.stdout | 0 tests/semaphore/lock.c | 29 +++++++++++++++++++ 5 files changed, 77 insertions(+), 32 deletions(-) create mode 100644 tests/expected/semaphore/lock.stdout create mode 100644 tests/semaphore/lock.c diff --git a/src/header/semaphore/mod.rs b/src/header/semaphore/mod.rs index 763301d7af..b38d3cf5d4 100644 --- a/src/header/semaphore/mod.rs +++ b/src/header/semaphore/mod.rs @@ -3,8 +3,15 @@ //! See . use crate::{ - header::time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec}, - platform::types::{c_char, c_int, c_long, c_uint, clockid_t}, + error::ResultExt, + header::{ + errno, + time::{self, timespec}, + }, + platform::{ + self, + types::{c_char, c_int, c_long, c_uint, clockid_t}, + }, }; /// See . @@ -67,9 +74,12 @@ pub unsafe extern "C" fn sem_post(sem: *mut sem_t) -> c_int { /// See . #[unsafe(no_mangle)] pub unsafe extern "C" fn sem_trywait(sem: *mut sem_t) -> c_int { - unsafe { get(sem) }.try_wait(); - - 0 + if unsafe { get(sem) }.try_wait() { + 0 + } else { + platform::ERRNO.set(errno::EAGAIN); + -1 + } } /// See . @@ -81,9 +91,10 @@ pub unsafe extern "C" fn sem_unlink(name: *const c_char) -> c_int { /// See . #[unsafe(no_mangle)] pub unsafe extern "C" fn sem_wait(sem: *mut sem_t) -> c_int { - if let Ok(()) = unsafe { get(sem) }.wait(None, CLOCK_MONOTONIC) {}; // TODO handle error - - 0 + unsafe { get(sem) } + .wait(None, time::CLOCK_MONOTONIC) + .map(|_| 0) + .or_minus_one_errno() } /// See . @@ -93,18 +104,19 @@ pub unsafe extern "C" fn sem_clockwait( clock_id: clockid_t, abstime: *const timespec, ) -> c_int { - if let Ok(()) = unsafe { get(sem) }.wait(Some(&unsafe { (*abstime).clone() }), clock_id) {}; // TODO handle error - - 0 + unsafe { get(sem) } + .wait(Some(&unsafe { (*abstime).clone() }), clock_id) + .map(|_| 0) + .or_minus_one_errno() } /// See . #[unsafe(no_mangle)] pub unsafe extern "C" fn sem_timedwait(sem: *mut sem_t, abstime: *const timespec) -> c_int { - if let Ok(()) = unsafe { get(sem) }.wait(Some(&unsafe { (*abstime).clone() }), CLOCK_REALTIME) { - }; // TODO handle error - - 0 + unsafe { get(sem) } + .wait(Some(&unsafe { (*abstime).clone() }), time::CLOCK_REALTIME) + .map(|_| 0) + .or_minus_one_errno() } unsafe fn get<'any>(sem: *mut sem_t) -> &'any RlctSempahore { diff --git a/src/sync/semaphore.rs b/src/sync/semaphore.rs index 55cb803f36..37fc502c8e 100644 --- a/src/sync/semaphore.rs +++ b/src/sync/semaphore.rs @@ -2,8 +2,16 @@ //TODO: improve implementation use crate::{ - header::time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec, timespec_realtime_to_monotonic}, - platform::types::{c_uint, clockid_t}, + error::{Errno, Result}, + header::{ + errno, + time::{CLOCK_MONOTONIC, CLOCK_REALTIME, timespec, timespec_realtime_to_monotonic}, + }, + platform::{ + Pal, Sys, + types::{c_uint, clockid_t}, + }, + sync::FutexAtomicTy, }; use core::sync::atomic::{AtomicU32, Ordering}; @@ -27,12 +35,12 @@ impl Semaphore { crate::sync::futex_wake(&self.count, i32::MAX); } - pub fn try_wait(&self) -> u32 { + pub fn try_wait(&self) -> bool { loop { let value = self.count.load(Ordering::SeqCst); if value == 0 { - return 0; + return false; } if self @@ -41,34 +49,29 @@ impl Semaphore { .is_ok() { // Acquired - return value; + return true; } // Try again (as long as value > 0) } } - pub fn wait(&self, timeout_opt: Option<×pec>, clock_id: clockid_t) -> Result<(), ()> { + pub fn wait(&self, timeout_opt: Option<×pec>, clock_id: clockid_t) -> Result<()> { loop { - let value = self.try_wait(); - - if value == 0 { + if self.try_wait() { return Ok(()); } - + // value must be zero if let Some(timeout) = timeout_opt { let relative = match clock_id { // FUTEX expect monotonic clock CLOCK_MONOTONIC => timeout.clone(), - CLOCK_REALTIME => match timespec_realtime_to_monotonic(timeout) { - Ok(relative) => relative, - Err(_) => return Err(()), - }, - _ => return Err(()), + CLOCK_REALTIME => timespec_realtime_to_monotonic(timeout)?, + _ => return Err(Errno(errno::EINVAL)), }; - crate::sync::futex_wait(&self.count, value, Some(&relative)); + unsafe { Sys::futex_wait(self.count.ptr(), 0, Some(&relative))? }; } else { // Use futex to wait for the next change, without a timeout - crate::sync::futex_wait(&self.count, value, None); + unsafe { Sys::futex_wait(self.count.ptr(), 0, None)? }; } } } diff --git a/tests/Makefile.tests.mk b/tests/Makefile.tests.mk index 9ff38126ad..16b7cda30f 100644 --- a/tests/Makefile.tests.mk +++ b/tests/Makefile.tests.mk @@ -70,6 +70,7 @@ EXPECT_NAMES=\ locale/setlocale \ math \ regex \ + semaphore/lock \ setjmp \ sigaction \ sigaltstack \ diff --git a/tests/expected/semaphore/lock.stdout b/tests/expected/semaphore/lock.stdout new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/semaphore/lock.c b/tests/semaphore/lock.c new file mode 100644 index 0000000000..6ad5d80c99 --- /dev/null +++ b/tests/semaphore/lock.c @@ -0,0 +1,29 @@ +#include +#include + +#include "test_helpers.h" + +int main(void) +{ + sem_t sem; + int status; + + status = sem_init(&sem, 0, 1); + ERROR_IF(sem_init, status, == -1); + + status = sem_trywait(&sem); + ERROR_IF(sem_trywait, status, == -1); + + status = sem_trywait(&sem); + UNEXP_IF(sem_trywait, status, != -1); + UNEXP_IF(errno, errno, != EAGAIN); + + status = sem_post(&sem); + ERROR_IF(sem_post, status, == -1); + + status = sem_wait(&sem); + ERROR_IF(sem_wait, status, == -1); + + status = sem_destroy(&sem); + ERROR_IF(sem_destroy, status, == -1); +} \ No newline at end of file