[libvirt PATCH 6/7] virMutex*: Warn on error
Daniel P. Berrangé
berrange at redhat.com
Thu Aug 5 13:58:03 UTC 2021
On Thu, Aug 05, 2021 at 03:33:57PM +0200, Tim Wiederhake wrote:
> On Thu, 2021-08-05 at 14:24 +0100, Daniel P. Berrangé wrote:
> > On Thu, Aug 05, 2021 at 03:08:50PM +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 and already
> > > unlocked mutex.
> >
> > We ignore the errors because in practice we found impls don't return
> > any error code when using PTHREAD_MUTEX_NORMAL - it would require use
> > of PTHREAD_MUTEX_ERRORCHECK.
> >
> > Did you actually see errors returned for real ?
>
> Yes. Please note that I apprently copy-pasted the wrong link in the
> cover letter. The pipeline for this series is correct
> (https://gitlab.com/twiederh/libvirt/-/pipelines/348505233), the
> pipeline for only patches 5, 6, 7 that demonstrate the non-zero return
> values of pthread_mutex_{lock,unlock,destroy} is here:
>
> https://gitlab.com/twiederh/libvirt/-/pipelines/348505770
All that's showing is
(killed by signal 6 SIGABRT)
where's the VIR_WARN output that ought to have ended up on stderr
from the failed test before aborting ?
> > > Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> > > ---
> > > 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 e89c1a09fb..f64dbee9e9 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);
> > > + }
> > > }
> > >
> > >
> > > --
> > > 2.31.1
> > >
> >
> > Regards,
> > Daniel
>
>
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 :|
More information about the libvir-list
mailing list