Fix semaphore error handling

Note that existing code which uses zix_sem_try_wait() may still compile against
this change, but be incorrect!

7 files changed, 96 insertions, 43 deletions

diff --git a/NEWS b/NEWS
index 707efab..1fba679 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,5 @@
-zix (0.1.0) unstable; urgency=medium
+zix (0.2.0) unstable; urgency=medium
 
   * Initial release
 
- -- David Robillard <d@drobilla.net>  Thu, 18 Aug 2022 16:50:12 +0000
+ -- David Robillard <d@drobilla.net>  Thu, 18 Aug 2022 21:51:42 +0000
diff --git a/include/zix/common.h b/include/zix/common.h
index c0944b5..2d5133f 100644
--- a/include/zix/common.h
+++ b/include/zix/common.h
@@ -25,7 +25,9 @@ typedef enum {
   ZIX_STATUS_EXISTS,
   ZIX_STATUS_BAD_ARG,
   ZIX_STATUS_BAD_PERMS,
-  ZIX_STATUS_REACHED_END
+  ZIX_STATUS_REACHED_END,
+  ZIX_STATUS_TIMEOUT,
+  ZIX_STATUS_OVERFLOW,
 } ZixStatus;
 
 /// Return a string describing a status code
diff --git a/include/zix/sem.h b/include/zix/sem.h
index 3549e02..b782761 100644
--- a/include/zix/sem.h
+++ b/include/zix/sem.h
@@ -51,26 +51,41 @@ struct ZixSemImpl;
 */
 typedef struct ZixSemImpl ZixSem;
 
-/// Create and initialize `sem` to `initial`
+/**
+   Create `sem` with the given `initial` value.
+
+   @return #ZIX_STATUS_SUCCESS, or an unlikely error.
+*/
 static inline ZixStatus
 zix_sem_init(ZixSem* ZIX_NONNULL sem, unsigned initial);
 
-/// Destroy `sem`
-static inline void
+/**
+   Destroy `sem`.
+
+   @return #ZIX_STATUS_SUCCESS, or an error.
+*/
+static inline ZixStatus
 zix_sem_destroy(ZixSem* ZIX_NONNULL sem);
 
 /**
    Increment and signal any waiters.
 
    Realtime safe.
+
+   @return #ZIX_STATUS_SUCCESS if `sem` was incremented, #ZIX_STATUS_OVERFLOW
+   if the maximum possible value would have been exceeded, or
+   #ZIX_STATUS_BAD_ARG if `sem` is invalid.
 */
-static inline void
+static inline ZixStatus
 zix_sem_post(ZixSem* ZIX_NONNULL sem);
 
 /**
    Wait until count is > 0, then decrement.
 
    Obviously not realtime safe.
+
+   @return #ZIX_STATUS_SUCCESS if `sem` was decremented, or #ZIX_STATUS_BAD_ARG
+   if `sem` is invalid.
 */
 static inline ZixStatus
 zix_sem_wait(ZixSem* ZIX_NONNULL sem);
@@ -78,9 +93,10 @@ zix_sem_wait(ZixSem* ZIX_NONNULL sem);
 /**
    Non-blocking version of wait().
 
-   @return true if decrement was successful (lock was acquired).
+   @return #ZIX_STATUS_SUCCESS if `sem` was decremented, #ZIX_STATUS_TIMEOUT if
+   it was already zero, or #ZIX_STATUS_BAD_ARG if `sem` is invalid.
 */
-static inline bool
+static inline ZixStatus
 zix_sem_try_wait(ZixSem* ZIX_NONNULL sem);
 
 /**
@@ -102,32 +118,39 @@ zix_sem_init(ZixSem* ZIX_NONNULL sem, unsigned val)
            : ZIX_STATUS_SUCCESS;
 }
 
-static inline void
+static inline ZixStatus
 zix_sem_destroy(ZixSem* ZIX_NONNULL sem)
 {
-  semaphore_destroy(mach_task_self(), sem->sem);
+  return semaphore_destroy(mach_task_self(), sem->sem) ? ZIX_STATUS_ERROR
+                                                       : ZIX_STATUS_SUCCESS;
 }
 
-static inline void
+static inline ZixStatus
 zix_sem_post(ZixSem* ZIX_NONNULL sem)
 {
-  semaphore_signal(sem->sem);
+  return semaphore_signal(sem->sem) ? ZIX_STATUS_ERROR : ZIX_STATUS_SUCCESS;
 }
 
 static inline ZixStatus
 zix_sem_wait(ZixSem* ZIX_NONNULL sem)
 {
-  if (semaphore_wait(sem->sem) != KERN_SUCCESS) {
-    return ZIX_STATUS_ERROR;
+  kern_return_t r = 0;
+  while ((r = semaphore_wait(sem->sem)) && r == KERN_ABORTED) {
+    // Interrupted, try again
   }
-  return ZIX_STATUS_SUCCESS;
+
+  return r ? ZIX_STATUS_ERROR : ZIX_STATUS_SUCCESS;
 }
 
-static inline bool
+static inline ZixStatus
 zix_sem_try_wait(ZixSem* ZIX_NONNULL sem)
 {
   const mach_timespec_t zero = {0, 0};
-  return semaphore_timedwait(sem->sem, zero) == KERN_SUCCESS;
+  const kern_return_t   r    = semaphore_timedwait(sem->sem, zero);
+
+  return (r == KERN_SUCCESS)               ? ZIX_STATUS_SUCCESS
+         : (r == KERN_OPERATION_TIMED_OUT) ? ZIX_STATUS_TIMEOUT
+                                           : ZIX_STATUS_ERROR;
 }
 
 #elif defined(_WIN32)
@@ -140,34 +163,38 @@ static inline ZixStatus
 zix_sem_init(ZixSem* ZIX_NONNULL sem, unsigned initial)
 {
   sem->sem = CreateSemaphore(NULL, (LONG)initial, LONG_MAX, NULL);
-  return (sem->sem) ? ZIX_STATUS_SUCCESS : ZIX_STATUS_ERROR;
+  return sem->sem ? ZIX_STATUS_SUCCESS : ZIX_STATUS_ERROR;
 }
 
-static inline void
+static inline ZixStatus
 zix_sem_destroy(ZixSem* ZIX_NONNULL sem)
 {
-  CloseHandle(sem->sem);
+  return CloseHandle(sem->sem) ? ZIX_STATUS_SUCCESS : ZIX_STATUS_ERROR;
 }
 
-static inline void
+static inline ZixStatus
 zix_sem_post(ZixSem* ZIX_NONNULL sem)
 {
-  ReleaseSemaphore(sem->sem, 1, NULL);
+  return ReleaseSemaphore(sem->sem, 1, NULL) ? ZIX_STATUS_SUCCESS
+                                             : ZIX_STATUS_ERROR;
 }
 
 static inline ZixStatus
 zix_sem_wait(ZixSem* ZIX_NONNULL sem)
 {
-  if (WaitForSingleObject(sem->sem, INFINITE) != WAIT_OBJECT_0) {
-    return ZIX_STATUS_ERROR;
-  }
-  return ZIX_STATUS_SUCCESS;
+  return WaitForSingleObject(sem->sem, INFINITE) == WAIT_OBJECT_0
+           ? ZIX_STATUS_SUCCESS
+           : ZIX_STATUS_ERROR;
 }
 
-static inline bool
+static inline ZixStatus
 zix_sem_try_wait(ZixSem* ZIX_NONNULL sem)
 {
-  return WaitForSingleObject(sem->sem, 0) == WAIT_OBJECT_0;
+  const DWORD r = WaitForSingleObject(sem->sem, 0);
+
+  return (r == WAIT_OBJECT_0)  ? ZIX_STATUS_SUCCESS
+         : (r == WAIT_TIMEOUT) ? ZIX_STATUS_TIMEOUT
+                               : ZIX_STATUS_ERROR;
 }
 
 #else /* !defined(__APPLE__) && !defined(_WIN32) */
@@ -179,36 +206,43 @@ struct ZixSemImpl {
 static inline ZixStatus
 zix_sem_init(ZixSem* ZIX_NONNULL sem, unsigned initial)
 {
-  return sem_init(&sem->sem, 0, initial) ? ZIX_STATUS_ERROR
+  return sem_init(&sem->sem, 0, initial) ? zix_errno_status(errno)
                                          : ZIX_STATUS_SUCCESS;
 }
 
-static inline void
+static inline ZixStatus
 zix_sem_destroy(ZixSem* ZIX_NONNULL sem)
 {
-  sem_destroy(&sem->sem);
+  return sem_destroy(&sem->sem) ? zix_errno_status(errno) : ZIX_STATUS_SUCCESS;
 }
 
-static inline void
+static inline ZixStatus
 zix_sem_post(ZixSem* ZIX_NONNULL sem)
 {
-  sem_post(&sem->sem);
+  return sem_post(&sem->sem) ? zix_errno_status(errno) : ZIX_STATUS_SUCCESS;
 }
 
 static inline ZixStatus
 zix_sem_wait(ZixSem* ZIX_NONNULL sem)
 {
-  while (sem_wait(&sem->sem) && errno == EINTR) {
+  int r = 0;
+  while ((r = sem_wait(&sem->sem)) && errno == EINTR) {
     // Interrupted, try again
   }
 
-  return zix_errno_status(errno);
+  return r ? zix_errno_status(errno) : ZIX_STATUS_SUCCESS;
 }
 
-static inline bool
+static inline ZixStatus
 zix_sem_try_wait(ZixSem* ZIX_NONNULL sem)
 {
-  return (sem_trywait(&sem->sem) == 0);
+  int r = 0;
+  while ((r = sem_trywait(&sem->sem)) && errno == EINTR) {
+    // Interrupted, try again
+  }
+
+  return r ? (errno == EAGAIN ? ZIX_STATUS_TIMEOUT : zix_errno_status(errno))
+           : ZIX_STATUS_SUCCESS;
 }
 
 #endif
diff --git a/meson.build b/meson.build
index 2f10fbb..2f442f0 100644
--- a/meson.build
+++ b/meson.build
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: CC0-1.0 OR ISC
 
 project('zix', ['c'],
-        version: '0.1.0',
+        version: '0.2.0',
         license: 'ISC',
         meson_version: '>= 0.56.0',
         default_options: [
diff --git a/src/status.c b/src/status.c
index 2ca3fe8..cbb3d36 100644
--- a/src/status.c
+++ b/src/status.c
@@ -25,6 +25,10 @@ zix_strerror(const ZixStatus status)
     return "Bad permissions";
   case ZIX_STATUS_REACHED_END:
     return "Reached end";
+  case ZIX_STATUS_TIMEOUT:
+    return "Timeout";
+  case ZIX_STATUS_OVERFLOW:
+    return "Overflow";
   }
   return "Unknown error";
 }
diff --git a/test/test_sem.c b/test/test_sem.c
index 023b8ee..d43dac8 100644
--- a/test/test_sem.c
+++ b/test/test_sem.c
@@ -4,6 +4,7 @@
 #undef NDEBUG
 
 #include "zix/attributes.h"
+#include "zix/common.h"
 #include "zix/sem.h"
 #include "zix/thread.h"
 
@@ -33,13 +34,24 @@ writer(void* ZIX_UNUSED(arg))
   printf("Writer starting\n");
 
   for (unsigned i = 0; i < n_signals; ++i) {
-    zix_sem_post(&sem);
+    assert(!zix_sem_post(&sem));
   }
 
   printf("Writer finished\n");
   return ZIX_THREAD_RESULT;
 }
 
+static void
+test_try_wait(void)
+{
+  assert(!zix_sem_init(&sem, 0));
+  assert(zix_sem_try_wait(&sem) == ZIX_STATUS_TIMEOUT);
+  assert(!zix_sem_post(&sem));
+  assert(!zix_sem_try_wait(&sem));
+  assert(zix_sem_try_wait(&sem) == ZIX_STATUS_TIMEOUT);
+  assert(!zix_sem_destroy(&sem));
+}
+
 int
 main(int argc, char** argv)
 {
@@ -52,6 +64,8 @@ main(int argc, char** argv)
     n_signals = (unsigned)strtol(argv[1], NULL, 10);
   }
 
+  test_try_wait();
+
   printf("Testing %u signals...\n", n_signals);
 
   assert(!zix_sem_init(&sem, 0));
@@ -64,7 +78,6 @@ main(int argc, char** argv)
 
   assert(!zix_thread_join(reader_thread));
   assert(!zix_thread_join(writer_thread));
-
-  zix_sem_destroy(&sem);
+  assert(!zix_sem_destroy(&sem));
   return 0;
 }
diff --git a/test/test_status.c b/test/test_status.c
index eb11bc1..486c210 100644
--- a/test/test_status.c
+++ b/test/test_status.c
@@ -37,7 +37,7 @@ test_strerror(void)
   const char* msg = zix_strerror(ZIX_STATUS_SUCCESS);
   assert(!strcmp(msg, "Success"));
 
-  for (int i = ZIX_STATUS_ERROR; i <= ZIX_STATUS_REACHED_END; ++i) {
+  for (int i = ZIX_STATUS_ERROR; i <= ZIX_STATUS_OVERFLOW; ++i) {
     msg = zix_strerror((ZixStatus)i);
     assert(strcmp(msg, "Success"));
   }