On Wed, Jul 03, 2024 at 02:44:37PM +0200, Tim Wiederhake wrote:
> `pthread_mutex_destroy`, `pthread_mutex_lock` and `pthread_mutex_unlock`
> return an error code that is currently ignored.
>
> Add debug information if one of these operations failed, e.g. when there
> is an attempt to destroy a still locked mutex or unlock an already
> unlocked mutex. Both scenarios are considered undefined behavior.
>
> Signed-off-by: Tim Wiederhake <twiederh@xxxxxxxxxx>
> ---
> src/util/virthread.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/src/util/virthread.c b/src/util/virthread.c
> index 5422bb74fd..14116a2221 100644
> --- a/src/util/virthread.c
> +++ b/src/util/virthread.c
> @@ -35,7 +35,10 @@
>
> #include "viralloc.h"
> #include "virthreadjob.h"
> +#include "virlog.h"
>
> +#define VIR_FROM_THIS VIR_FROM_THREAD
> +VIR_LOG_INIT("util.thread");
>
> int virOnce(virOnceControl *once, virOnceFunc init)
> {
> @@ -83,17 +86,23 @@ int virMutexInitRecursive(virMutex *m)
>
> void virMutexDestroy(virMutex *m)
> {
> - pthread_mutex_destroy(&m->lock);
> + if (pthread_mutex_destroy(&m->lock)) {
> + VIR_WARN("Failed to destroy mutex=%p", m);
> + }
> }
>
> void virMutexLock(virMutex *m)
> {
> - pthread_mutex_lock(&m->lock);
> + if (pthread_mutex_lock(&m->lock)) {
> + VIR_WARN("Failed to lock mutex=%p", m);
> + }
> }
>
> void virMutexUnlock(virMutex *m)
> {
> - pthread_mutex_unlock(&m->lock);
> + if (pthread_mutex_unlock(&m->lock)) {
> + VIR_WARN("Failed to unlock mutex=%p", m);
> + }
> }
I'd be surprised if these lock/unlock warnings ever trigger, since IIUC
they would need us to be using an error checking mutex, not a regular
mutex. IOW, aren't these just adding condition test overhead + unreachable
code to the lock calls ?
The 2nd patch shows failures in the destroy calls IIUC.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
