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
This commit is contained in:
Oliver Jowett 2020-11-17 19:59:59 +08:00
parent 5c043e8496
commit c1eeda612e
1 changed files with 12 additions and 8 deletions

20
fifo.c
View File

@ -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
}
}