From c1eeda612e2c27f4873652183a482481a1a2d8f3 Mon Sep 17 00:00:00 2001 From: Oliver Jowett Date: Tue, 17 Nov 2020 19:59:59 +0800 Subject: [PATCH] fifo_acquire / fifo_dequeue: maybe fix pthread_cond_timedwait error handling It was possible, due to an off-by-one error in normalize_timespec, that the computed deadline passed to pthread_cond_timedwait had tv_nsec == 1000000000. glibc would reject this with EINVAL. While I never caught this in the act, there seem to be two follow-on problems from this: 1) we were looking for a negative return from pthread_cond_timedwait and testing errno. This is not how errors are reported, instead pthread functions return the error value directly. So the enclosing loop could spin forever, repeatedly passing the bad deadline value. 2) even if the error handling caught the EINVAL return, it assumed that the mutex was no longer held and didn't release it; but on error returns, the mutex state is actually untouched. Fix both cases (probably), and log about unusual returns from pthread_cond_timedwait --- fifo.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/fifo.c b/fifo.c index f7675f2..e33a4e2 100644 --- a/fifo.c +++ b/fifo.c @@ -140,11 +140,13 @@ struct mag_buf *fifo_acquire(uint32_t timeout_ms) } // No free buffers, wait for one - if (pthread_cond_timedwait(&fifo_free_cond, &fifo_mutex, &deadline) < 0) { - if (errno != ETIMEDOUT) - return NULL; // unexpected error, mutex no longer held! + int err = pthread_cond_timedwait(&fifo_free_cond, &fifo_mutex, &deadline); + if (err) { + if (err != ETIMEDOUT) { + fprintf(stderr, "fifo_acquire: pthread_cond_timedwait unexpectedly returned %s\n", strerror(err)); + } - goto done; // timed out + goto done; // done waiting } } @@ -219,11 +221,13 @@ struct mag_buf *fifo_dequeue(uint32_t timeout_ms) } // No data pending, wait for some - if (pthread_cond_timedwait(&fifo_notempty_cond, &fifo_mutex, &deadline) < 0) { - if (errno != ETIMEDOUT) - return NULL; // unexpected error, mutex no longer held! + int err = pthread_cond_timedwait(&fifo_notempty_cond, &fifo_mutex, &deadline); + if (err) { + if (err != ETIMEDOUT) { + fprintf(stderr, "fifo_dequeue: pthread_cond_timedwait unexpectedly returned %s\n", strerror(err)); + } - goto done; // timed out + goto done; // done waiting } }