r4549 - trunk/varnish-cache/bin/varnishd
phk at projects.linpro.no
phk at projects.linpro.no
Wed Feb 10 13:48:18 CET 2010
Author: phk
Date: 2010-02-10 13:48:17 +0100 (Wed, 10 Feb 2010)
New Revision: 4549
Modified:
trunk/varnish-cache/bin/varnishd/hash_critbit.c
Log:
Close what might be a race, and assert that it doesn't happen:
When we do a lookup, we first try without the lock, if this gives
us a refcnt==0 objhead, we try again, with the lock.
When we deref an objhead, we would decrement the refcnt, holding oh->mtx,
then lock the critbit-lock and remove it from the tree.
It might be possible for a locked lookup to find an oh we just decremented
the refcnt off, and we did not check for refcnt==0 in that case.
Fix this, by removing the oh from the tree, holding the critbit-lock,
before decrementing the refcnt. This way, a locked lookup can never
find a refcnt==0 oh in the tree, and the unlocked lookup still catches
this with the refcnt==0 check.
Modified: trunk/varnish-cache/bin/varnishd/hash_critbit.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/hash_critbit.c 2010-02-10 12:44:42 UTC (rev 4548)
+++ trunk/varnish-cache/bin/varnishd/hash_critbit.c 2010-02-10 12:48:17 UTC (rev 4549)
@@ -216,6 +216,7 @@
/* We found a node, does it match ? */
oh2 = hcb_l_node(pp);
+ CHECK_OBJ_NOTNULL(oh2, OBJHEAD_MAGIC);
if (!memcmp(oh2->digest, oh->digest, DIGEST_LEN))
return (oh2);
@@ -357,6 +358,8 @@
(void)sleep(1);
Lck_Lock(&hcb_mtx);
VTAILQ_FOREACH_SAFE(oh, &laylow, coollist, oh2) {
+ CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
+ AZ(oh->refcnt);
y = (void *)&oh->u;
if (y->leaf[0] || y->leaf[1])
continue;
@@ -365,7 +368,6 @@
#ifdef PHK
fprintf(stderr, "OH %p is cold enough\n", oh);
#endif
- AZ(oh->refcnt);
HSH_DeleteObjHead(&ww, oh);
}
}
@@ -401,7 +403,8 @@
CHECK_OBJ_NOTNULL(oh, OBJHEAD_MAGIC);
Lck_Lock(&oh->mtx);
assert(oh->refcnt > 0);
- if (--oh->refcnt == 0) {
+ if (oh->refcnt == 1) {
+ /* Remove from tree before decrementing refcnt to zero */
Lck_Lock(&hcb_mtx);
hcb_delete(&hcb_root, oh);
assert(VTAILQ_EMPTY(&oh->objcs));
@@ -410,6 +413,7 @@
VTAILQ_INSERT_TAIL(&laylow, oh, coollist);
Lck_Unlock(&hcb_mtx);
}
+ oh->refcnt--;
Lck_Unlock(&oh->mtx);
#ifdef PHK
fprintf(stderr, "hcb_defef %d %d <%s>\n", __LINE__, r, oh->hash);
@@ -452,6 +456,7 @@
oh = hcb_insert(&hcb_root, noh, 1);
if (oh == noh) {
VSL_stats->hcb_insert++;
+ assert(oh->refcnt > 0);
#ifdef PHK
fprintf(stderr, "hcb_lookup %d\n", __LINE__);
#endif
@@ -462,6 +467,7 @@
fprintf(stderr, "hcb_lookup %d\n", __LINE__);
#endif
Lck_Lock(&oh->mtx);
+ assert(oh->refcnt > 0);
oh->refcnt++;
Lck_Unlock(&oh->mtx);
}
More information about the varnish-commit
mailing list