Commit 7f625bbe authored by Yoni Fogel's avatar Yoni Fogel

Addresses #1691 Further fixes to a race condition where refcounts in a...

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
parent d63cc559
...@@ -266,6 +266,13 @@ static void cachefile_init_filenum(CACHEFILE cf, int fd, const char *fname, stru ...@@ -266,6 +266,13 @@ static void cachefile_init_filenum(CACHEFILE cf, int fd, const char *fname, stru
cf->fileid = fileid; cf->fileid = fileid;
cf->fname = fname ? toku_strdup(fname) : 0; 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. // 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) { 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 ...@@ -277,12 +284,14 @@ int toku_cachetable_openfd (CACHEFILE *cfptr, CACHETABLE ct, int fd, const char
r=errno; close(fd); r=errno; close(fd);
return r; return r;
} }
cachetable_lock(ct);
for (extant = ct->cachefiles; extant; extant=extant->next) { for (extant = ct->cachefiles; extant; extant=extant->next) {
if (memcmp(&extant->fileid, &fileid, sizeof(fileid))==0) { if (memcmp(&extant->fileid, &fileid, sizeof(fileid))==0) {
r = close(fd); r = close(fd);
assert(r == 0); assert(r == 0);
extant->refcount++; cachefile_refup(extant);
*cfptr = extant; *cfptr = extant;
cachetable_unlock(ct);
return 0; return 0;
} }
} }
...@@ -309,10 +318,12 @@ int toku_cachetable_openfd (CACHEFILE *cfptr, CACHETABLE ct, int fd, const char ...@@ -309,10 +318,12 @@ int toku_cachetable_openfd (CACHEFILE *cfptr, CACHETABLE ct, int fd, const char
newcf->end_checkpoint_userdata = 0; newcf->end_checkpoint_userdata = 0;
*cfptr = newcf; *cfptr = newcf;
cachetable_unlock(ct);
return 0; return 0;
} }
} }
//TEST_ONLY_FUNCTION
int toku_cachetable_openf (CACHEFILE *cfptr, CACHETABLE ct, const char *fname, int flags, mode_t mode) { int toku_cachetable_openf (CACHEFILE *cfptr, CACHETABLE ct, const char *fname, int flags, mode_t mode) {
int fd = open(fname, flags+O_BINARY, mode); int fd = open(fname, flags+O_BINARY, mode);
if (fd<0) return errno; if (fd<0) return errno;
...@@ -388,11 +399,6 @@ static CACHEFILE remove_cf_from_list (CACHEFILE cf, CACHEFILE list) { ...@@ -388,11 +399,6 @@ static CACHEFILE remove_cf_from_list (CACHEFILE cf, CACHEFILE list) {
static int cachetable_flush_cachefile (CACHETABLE, CACHEFILE cf); 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) { int toku_cachefile_close (CACHEFILE *cfp, TOKULOGGER logger, char **error_string) {
CACHEFILE cf = *cfp; CACHEFILE cf = *cfp;
...@@ -1412,7 +1418,7 @@ toku_cachetable_begin_checkpoint (CACHETABLE ct, TOKULOGGER logger) { ...@@ -1412,7 +1418,7 @@ toku_cachetable_begin_checkpoint (CACHETABLE ct, TOKULOGGER logger) {
if (cf->refcount>0) { if (cf->refcount>0) {
//Incremement reference count of cachefile because we're using it for the checkpoint. //Incremement reference count of cachefile because we're using it for the checkpoint.
//This will prevent closing during the checkpoint. //This will prevent closing during the checkpoint.
toku_cachefile_refup(cf); cachefile_refup(cf);
cf->next_in_checkpoint = ct->cachefiles_in_checkpoint; cf->next_in_checkpoint = ct->cachefiles_in_checkpoint;
ct->cachefiles_in_checkpoint = cf; ct->cachefiles_in_checkpoint = cf;
cf->for_checkpoint = TRUE; cf->for_checkpoint = TRUE;
......
...@@ -162,9 +162,6 @@ int toku_cachefile_close (CACHEFILE*, TOKULOGGER, char **error_string); ...@@ -162,9 +162,6 @@ int toku_cachefile_close (CACHEFILE*, TOKULOGGER, char **error_string);
// Returns: 0 if success, otherwise returns an error number. // Returns: 0 if success, otherwise returns an error number.
int toku_cachefile_flush (CACHEFILE); 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) // Return on success (different from pread and pwrite)
//int cachefile_pwrite (CACHEFILE, const void *buf, size_t count, toku_off_t offset); //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); //int cachefile_pread (CACHEFILE, void *buf, size_t count, toku_off_t offset);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment