From 46d1cde8f598c931b82541b8db257de89707fc0b Mon Sep 17 00:00:00 2001 From: Yoni Fogel Date: Tue, 16 Apr 2013 23:57:50 -0400 Subject: [PATCH] Addresses #1691 Further fixes to a race condition where refcounts in a cachefile were not protected by any lock. See [11371] removed most of the causes of this (by limiting the places where refcount was edited). Now (cachefile) refcounts are protected by the cachetable_lock. git-svn-id: file:///svn/toku/tokudb@11408 c7de825b-a66e-492c-adef-691d508d4ae1 --- newbrt/cachetable.c | 20 +++++++++++++------- newbrt/cachetable.h | 3 --- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/newbrt/cachetable.c b/newbrt/cachetable.c index b6a36389105..b41d18578cf 100644 --- a/newbrt/cachetable.c +++ b/newbrt/cachetable.c @@ -266,6 +266,13 @@ static void cachefile_init_filenum(CACHEFILE cf, int fd, const char *fname, stru cf->fileid = fileid; cf->fname = fname ? toku_strdup(fname) : 0; } +// +// Increment the reference count +// MUST HOLD cachetable lock +static void +cachefile_refup (CACHEFILE cf) { + cf->refcount++; +} // If something goes wrong, close the fd. After this, the caller shouldn't close the fd, but instead should close the cachefile. int toku_cachetable_openfd (CACHEFILE *cfptr, CACHETABLE ct, int fd, const char *fname) { @@ -277,12 +284,14 @@ int toku_cachetable_openfd (CACHEFILE *cfptr, CACHETABLE ct, int fd, const char r=errno; close(fd); return r; } + cachetable_lock(ct); for (extant = ct->cachefiles; extant; extant=extant->next) { if (memcmp(&extant->fileid, &fileid, sizeof(fileid))==0) { r = close(fd); assert(r == 0); - extant->refcount++; + cachefile_refup(extant); *cfptr = extant; + cachetable_unlock(ct); return 0; } } @@ -309,10 +318,12 @@ int toku_cachetable_openfd (CACHEFILE *cfptr, CACHETABLE ct, int fd, const char newcf->end_checkpoint_userdata = 0; *cfptr = newcf; + cachetable_unlock(ct); return 0; } } +//TEST_ONLY_FUNCTION int toku_cachetable_openf (CACHEFILE *cfptr, CACHETABLE ct, const char *fname, int flags, mode_t mode) { int fd = open(fname, flags+O_BINARY, mode); if (fd<0) return errno; @@ -388,11 +399,6 @@ static CACHEFILE remove_cf_from_list (CACHEFILE cf, CACHEFILE list) { static int cachetable_flush_cachefile (CACHETABLE, CACHEFILE cf); -// Increment the reference count -void toku_cachefile_refup (CACHEFILE cf) { - cf->refcount++; -} - int toku_cachefile_close (CACHEFILE *cfp, TOKULOGGER logger, char **error_string) { CACHEFILE cf = *cfp; @@ -1412,7 +1418,7 @@ toku_cachetable_begin_checkpoint (CACHETABLE ct, TOKULOGGER logger) { if (cf->refcount>0) { //Incremement reference count of cachefile because we're using it for the checkpoint. //This will prevent closing during the checkpoint. - toku_cachefile_refup(cf); + cachefile_refup(cf); cf->next_in_checkpoint = ct->cachefiles_in_checkpoint; ct->cachefiles_in_checkpoint = cf; cf->for_checkpoint = TRUE; diff --git a/newbrt/cachetable.h b/newbrt/cachetable.h index 9a93ca33e29..e0359ecc75d 100644 --- a/newbrt/cachetable.h +++ b/newbrt/cachetable.h @@ -162,9 +162,6 @@ int toku_cachefile_close (CACHEFILE*, TOKULOGGER, char **error_string); // Returns: 0 if success, otherwise returns an error number. int toku_cachefile_flush (CACHEFILE); -// Increment the reference count. Use close to decrement it. -void toku_cachefile_refup (CACHEFILE cfp); - // Return on success (different from pread and pwrite) //int cachefile_pwrite (CACHEFILE, const void *buf, size_t count, toku_off_t offset); //int cachefile_pread (CACHEFILE, void *buf, size_t count, toku_off_t offset);