Re: [PATCH 1/2] virMutex: Warn on error

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(a)redhat.com&gt; > --- >  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