diff --git a/CHANGES b/CHANGES index 1f0cfaf337..4f7ace543a 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +2330. [bug] Remove potential race condition when handling + over memory events. [RT #17572] + + WARNING: API CHANGE: over memory callback + function now needs to call isc_mem_waterack(). + See for details. + 2329. [bug] Clearer help text for dig's '-x' and '-i' options. 2328. [bug] Add AAAA addresses for A.ROOT-SERVERS.NET, diff --git a/lib/dns/acache.c b/lib/dns/acache.c index c95e203663..4472c4f90d 100644 --- a/lib/dns/acache.c +++ b/lib/dns/acache.c @@ -14,7 +14,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: acache.c,v 1.20 2007/06/19 23:47:16 tbox Exp $ */ +/* $Id: acache.c,v 1.21 2008/02/07 02:41:26 marka Exp $ */ #include @@ -965,10 +965,14 @@ water(void *arg, int mark) { LOCK(&acache->cleaner.lock); - acache->cleaner.overmem = overmem; + if (acache->cleaner.overmem != overmem) { + acache->cleaner.overmem = overmem; - if (acache->cleaner.overmem_event != NULL) - isc_task_send(acache->task, &acache->cleaner.overmem_event); + if (acache->cleaner.overmem_event != NULL) + isc_task_send(acache->task, + &acache->cleaner.overmem_event); + isc_mem_waterack(acache->mctx, mark); + } UNLOCK(&acache->cleaner.lock); } diff --git a/lib/dns/adb.c b/lib/dns/adb.c index 182f1c5386..48cb9f988e 100644 --- a/lib/dns/adb.c +++ b/lib/dns/adb.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: adb.c,v 1.233 2007/10/19 17:15:53 explorer Exp $ */ +/* $Id: adb.c,v 1.234 2008/02/07 02:41:26 marka Exp $ */ /*! \file * @@ -128,6 +128,7 @@ struct dns_adb { isc_mutex_t lock; isc_mutex_t reflock; /*%< Covers irefcnt, erefcnt */ + isc_mutex_t overmemlock; /*%< Covers overmem */ isc_mem_t *mctx; dns_view_t *view; isc_timermgr_t *timermgr; @@ -2138,6 +2139,7 @@ destroy(dns_adb_t *adb) { DESTROYLOCK(&adb->reflock); DESTROYLOCK(&adb->lock); DESTROYLOCK(&adb->mplock); + DESTROYLOCK(&adb->overmemlock); isc_mem_putanddetach(&adb->mctx, adb, sizeof(dns_adb_t)); } @@ -2225,6 +2227,10 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr, if (result != ISC_R_SUCCESS) goto fail0d; + result = isc_mutex_init(&adb->overmemlock); + if (result != ISC_R_SUCCESS) + goto fail0e; + /* * Initialize the bucket locks for names and elements. * May as well initialize the list heads, too. @@ -2343,6 +2349,8 @@ dns_adb_create(isc_mem_t *mem, dns_view_t *view, isc_timermgr_t *timermgr, if (adb->afmp != NULL) isc_mempool_destroy(&adb->afmp); + DESTROYLOCK(&adb->overmemlock); + fail0e: DESTROYLOCK(&adb->reflock); fail0d: DESTROYLOCK(&adb->mplock); @@ -3782,16 +3790,25 @@ water(void *arg, int mark) { DP(ISC_LOG_DEBUG(1), "adb reached %s water mark", overmem ? "high" : "low"); - adb->overmem = overmem; + /* + * We can't use adb->lock as there is potential for water + * to be called when adb->lock is held. + */ + LOCK(&adb->overmemlock); + if (adb->overmem != overmem) { + adb->overmem = overmem; #if 0 /* we don't need this timer for the new cleaning policy. */ - if (overmem) { - isc_interval_t interval; + if (overmem) { + isc_interval_t interval; - isc_interval_set(&interval, 0, 1); - (void)isc_timer_reset(adb->timer, isc_timertype_once, NULL, - &interval, ISC_TRUE); - } + isc_interval_set(&interval, 0, 1); + (void)isc_timer_reset(adb->timer, isc_timertype_once, + NULL, &interval, ISC_TRUE); + } #endif + isc_mem_waterack(adb->mctx, mark); + } + UNLOCK(&adb->overmemlock); } void diff --git a/lib/dns/cache.c b/lib/dns/cache.c index cd38420509..fb03429347 100644 --- a/lib/dns/cache.c +++ b/lib/dns/cache.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: cache.c,v 1.76 2007/10/19 17:15:53 explorer Exp $ */ +/* $Id: cache.c,v 1.77 2008/02/07 02:41:26 marka Exp $ */ /*! \file */ @@ -708,8 +708,11 @@ water(void *arg, int mark) { LOCK(&cache->cleaner.lock); - dns_db_overmem(cache->db, overmem); - cache->cleaner.overmem = overmem; + if (overmem != cache->cleaner.overmem) { + dns_db_overmem(cache->db, overmem); + cache->cleaner.overmem = overmem; + isc_mem_waterack(cache->mctx, mark); + } UNLOCK(&cache->cleaner.lock); } diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 4a401ceb45..af8141a720 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: mem.h,v 1.75 2008/01/18 23:46:58 tbox Exp $ */ +/* $Id: mem.h,v 1.76 2008/02/07 02:41:26 marka Exp $ */ #ifndef ISC_MEM_H #define ISC_MEM_H 1 @@ -339,10 +339,27 @@ isc_mem_setwater(isc_mem_t *mctx, isc_mem_water_t water, void *water_arg, /*%< * Set high and low water marks for this memory context. * - * When the memory - * usage of 'mctx' exceeds 'hiwater', '(water)(water_arg, #ISC_MEM_HIWATER)' - * will be called. When the usage drops below 'lowater', 'water' will - * again be called, this time with #ISC_MEM_LOWATER. + * When the memory usage of 'mctx' exceeds 'hiwater', + * '(water)(water_arg, #ISC_MEM_HIWATER)' will be called. 'water' needs to + * call isc_mem_waterack() with #ISC_MEM_HIWATER to acknowlege the state + * change. 'water' may be called multiple times. + * + * When the usage drops below 'lowater', 'water' will again be called, this + * time with #ISC_MEM_LOWATER. 'water' need to calls isc_mem_waterack() with + * #ISC_MEM_LOWATER to acknowlege the change. + * + * static void + * water(void *arg, int mark) { + * struct foo *foo = *arg; + * + * LOCK(&foo->marklock); + * if (foo->mark != mark) { + * foo->mark = mark; + * .... + * isc_mem_waterack(foo->mctx, mark); + * } + * UNLOCK(&foo->marklock); + * } * * If 'water' is NULL then 'water_arg', 'hi_water' and 'lo_water' are * ignored and the state is reset. @@ -353,6 +370,12 @@ isc_mem_setwater(isc_mem_t *mctx, isc_mem_water_t water, void *water_arg, * hi_water >= lo_water */ +void +isc_mem_waterack(isc_mem_t *ctx, int mark); +/*%< + * Called to acknowledge changes in signalled by calls to 'water'. + */ + void isc_mem_printactive(isc_mem_t *mctx, FILE *file); /*%< diff --git a/lib/isc/mem.c b/lib/isc/mem.c index b949ce9020..4b14ad0ef9 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -15,7 +15,7 @@ * PERFORMANCE OF THIS SOFTWARE. */ -/* $Id: mem.c,v 1.140 2008/01/18 23:46:58 tbox Exp $ */ +/* $Id: mem.c,v 1.141 2008/02/07 02:41:26 marka Exp $ */ /*! \file */ @@ -1086,7 +1086,6 @@ isc__mem_get(isc_mem_t *ctx, size_t size FLARG) { ADD_TRACE(ctx, ptr, size, file, line); if (ctx->hi_water != 0U && !ctx->hi_called && ctx->inuse > ctx->hi_water) { - ctx->hi_called = ISC_TRUE; call_water = ISC_TRUE; } if (ctx->inuse > ctx->maxinuse) { @@ -1144,8 +1143,6 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) */ if (ctx->hi_called && (ctx->inuse < ctx->lo_water || ctx->lo_water == 0U)) { - ctx->hi_called = ISC_FALSE; - if (ctx->water != NULL) call_water = ISC_TRUE; } @@ -1155,6 +1152,18 @@ isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size FLARG) (ctx->water)(ctx->water_arg, ISC_MEM_LOWATER); } +void +isc_mem_waterack(isc_mem_t *ctx, int flag) { + REQUIRE(VALID_CONTEXT(ctx)); + + MCTXLOCK(ctx, &ctx->lock); + if (flag == ISC_MEM_LOWATER) + ctx->hi_called = ISC_FALSE; + else if (flag == ISC_MEM_HIWATER) + ctx->hi_called = ISC_TRUE; + MCTXUNLOCK(ctx, &ctx->lock); +} + #if ISC_MEM_TRACKLINES static void print_active(isc_mem_t *mctx, FILE *out) { diff --git a/lib/isc/win32/libisc.def b/lib/isc/win32/libisc.def index 42cb272b44..1480d85e92 100644 --- a/lib/isc/win32/libisc.def +++ b/lib/isc/win32/libisc.def @@ -255,6 +255,7 @@ isc_mem_setdestroycheck isc_mem_setquota isc_mem_setwater isc_mem_stats +isc_mem_waterack isc_mempool_associatelock isc_mempool_create isc_mempool_destroy