Hi, With Varnish locks available for VMODs in 4.1 I decided to convert my plain pthread locks on my 4.1-only module. I even found a place where I forgot a lock thanks to being able to assert on it being held :)
This patch aims at making this assertion a bit more useful: Assert error in Lck__Assert(), cache/cache_lck.c line 175: Condition(ilck->held) not true. The backtrace doesn't mention static function names or line numbers, so with the patch I got this instead: Assert error in vmod_dns_del(), vmod_named.c line 231: Condition(Lck__Held(&dir->mtx)) not true. Best, Dridi
From 4e6164b724c569d5d8ac50d3f7b8d43bf2d8ad5f Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune <[email protected]> Date: Thu, 25 Feb 2016 15:09:52 +0100 Subject: [PATCH] Make Lck_AssertHeld trigger in the caller code The `held == 0` branch in the Lck__AssertHeld function is never taken and `cache.h` instructs to only use the macro. Being generated from calling code, the panic message becomes more informative. --- bin/varnishd/cache/cache.h | 9 +++++++-- bin/varnishd/cache/cache_lck.c | 22 +++++++++++++--------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h index 08eaa2d..b7d8153 100644 --- a/bin/varnishd/cache/cache.h +++ b/bin/varnishd/cache/cache.h @@ -831,7 +831,8 @@ void Lck__Lock(struct lock *lck, const char *p, int l); void Lck__Unlock(struct lock *lck, const char *p, int l); int Lck__Trylock(struct lock *lck, const char *p, int l); void Lck__New(struct lock *lck, struct VSC_C_lck *, const char *); -void Lck__Assert(const struct lock *lck, int held); +int Lck__Held(const struct lock *lck); +int Lck__Owned(const struct lock *lck); /* public interface: */ void Lck_Delete(struct lock *lck); @@ -841,7 +842,11 @@ int Lck_CondWait(pthread_cond_t *cond, struct lock *lck, double); #define Lck_Lock(a) Lck__Lock(a, __func__, __LINE__) #define Lck_Unlock(a) Lck__Unlock(a, __func__, __LINE__) #define Lck_Trylock(a) Lck__Trylock(a, __func__, __LINE__) -#define Lck_AssertHeld(a) Lck__Assert(a, 1) +#define Lck_AssertHeld(a) \ + do { \ + assert(Lck__Held(a)); \ + assert(Lck__Owned(a)); \ + } while (0) struct VSC_C_lck *Lck_CreateClass(const char *name); diff --git a/bin/varnishd/cache/cache_lck.c b/bin/varnishd/cache/cache_lck.c index b702c2f..81547be 100644 --- a/bin/varnishd/cache/cache_lck.c +++ b/bin/varnishd/cache/cache_lck.c @@ -165,19 +165,23 @@ Lck__Trylock(struct lock *lck, const char *p, int l) return (r); } -void -Lck__Assert(const struct lock *lck, int held) +int +Lck__Held(const struct lock *lck) { struct ilck *ilck; CAST_OBJ_NOTNULL(ilck, lck->priv, ILCK_MAGIC); - if (held) { - assert(ilck->held); - assert(pthread_equal(ilck->owner, pthread_self())); - } else { - AZ(ilck->held); - AZ(pthread_equal(ilck->owner, pthread_self())); - } + return (ilck->held); +} + +int +Lck__Owned(const struct lock *lck) +{ + struct ilck *ilck; + + CAST_OBJ_NOTNULL(ilck, lck->priv, ILCK_MAGIC); + AN(ilck->held); + return (pthread_equal(ilck->owner, pthread_self())); } int __match_proto__() -- 2.5.0
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
