mirror of
https://github.com/s3fs-fuse/s3fs-fuse.git
synced 2024-12-22 08:48:55 +00:00
Increase FdEntity reference count when returning
Previously s3fs had a race condition where one client could delete FdEntity that another client was using. Add a simple concurrent test which previously failed but now succeeds. Fixes #964.
This commit is contained in:
parent
0d43d070cc
commit
f53503438c
@ -700,6 +700,9 @@ void FdEntity::Close()
|
||||
|
||||
if(0 < refcnt){
|
||||
refcnt--;
|
||||
}else{
|
||||
S3FS_PRN_EXIT("reference count underflow");
|
||||
abort();
|
||||
}
|
||||
if(0 == refcnt){
|
||||
if(!cachepath.empty()){
|
||||
@ -2058,6 +2061,7 @@ FdEntity* FdManager::GetFdEntity(const char* path, int existfd)
|
||||
|
||||
fdent_map_t::iterator iter = fent.find(string(path));
|
||||
if(fent.end() != iter && (-1 == existfd || (*iter).second->GetFd() == existfd)){
|
||||
iter->second->Dup();
|
||||
return (*iter).second;
|
||||
}
|
||||
|
||||
@ -2066,6 +2070,7 @@ FdEntity* FdManager::GetFdEntity(const char* path, int existfd)
|
||||
if((*iter).second && (*iter).second->GetFd() == existfd){
|
||||
// found opened fd in map
|
||||
if(0 == strcmp((*iter).second->GetPath(), path)){
|
||||
iter->second->Dup();
|
||||
return (*iter).second;
|
||||
}
|
||||
// found fd, but it is used another file(file descriptor is recycled)
|
||||
@ -2084,6 +2089,7 @@ FdEntity* FdManager::Open(const char* path, headers_t* pmeta, ssize_t size, time
|
||||
if(!path || '\0' == path[0]){
|
||||
return NULL;
|
||||
}
|
||||
bool close = false;
|
||||
FdEntity* ent;
|
||||
{
|
||||
AutoLock auto_lock(&FdManager::fd_manager_lock);
|
||||
@ -2107,6 +2113,8 @@ FdEntity* FdManager::Open(const char* path, headers_t* pmeta, ssize_t size, time
|
||||
if(fent.end() != iter){
|
||||
// found
|
||||
ent = (*iter).second;
|
||||
ent->Dup();
|
||||
close = true;
|
||||
|
||||
}else if(is_create){
|
||||
// not found
|
||||
@ -2142,6 +2150,9 @@ FdEntity* FdManager::Open(const char* path, headers_t* pmeta, ssize_t size, time
|
||||
if(0 != ent->Open(pmeta, size, time, no_fd_lock_wait)){
|
||||
return NULL;
|
||||
}
|
||||
if(close){
|
||||
ent->Close();
|
||||
}
|
||||
return ent;
|
||||
}
|
||||
|
||||
|
@ -225,6 +225,7 @@ class FdManager
|
||||
static void FreeReservedDiskSpace(size_t size);
|
||||
bool ReserveDiskSpace(size_t size);
|
||||
|
||||
// Return FdEntity associated with path, returning NULL on error. This operation increments the reference count; callers must decrement via Close after use.
|
||||
FdEntity* GetFdEntity(const char* path, int existfd = -1);
|
||||
FdEntity* Open(const char* path, headers_t* pmeta = NULL, ssize_t size = -1, time_t time = -1, bool force_tmpfile = false, bool is_create = true, bool no_fd_lock_wait = false);
|
||||
FdEntity* ExistOpen(const char* path, int existfd = -1, bool ignore_existfd = false);
|
||||
|
@ -2332,6 +2332,9 @@ static int s3fs_release(const char* _path, struct fuse_file_info* fi)
|
||||
if(ent->GetFd() != static_cast<int>(fi->fh)){
|
||||
S3FS_PRN_WARN("different fd(%d - %llu)", ent->GetFd(), (unsigned long long)(fi->fh));
|
||||
}
|
||||
|
||||
// Once for the implicit refcnt from GetFdEntity and again for release
|
||||
ent->Close();
|
||||
FdManager::get()->Close(ent);
|
||||
|
||||
// check - for debug
|
||||
|
@ -522,6 +522,20 @@ function test_overwrite_existing_file_range {
|
||||
rm -f ${TEST_TEXT_FILE}
|
||||
}
|
||||
|
||||
function test_concurrency {
|
||||
for i in `seq 10`; do echo foo > $i; done
|
||||
for process in `seq 2`; do
|
||||
for i in `seq 100`; do
|
||||
file=$(ls | sed -n "$(($RANDOM % 10 + 1)){p;q}")
|
||||
cat $file >/dev/null || true
|
||||
rm -f $file
|
||||
echo foo > $i || true
|
||||
done &
|
||||
done
|
||||
wait
|
||||
}
|
||||
|
||||
|
||||
function add_all_tests {
|
||||
add_tests test_append_file
|
||||
add_tests test_truncate_file
|
||||
@ -546,6 +560,7 @@ function add_all_tests {
|
||||
add_tests test_rm_rf_dir
|
||||
add_tests test_write_after_seek_ahead
|
||||
add_tests test_overwrite_existing_file_range
|
||||
add_tests test_concurrency
|
||||
}
|
||||
|
||||
init_suite
|
||||
|
Loading…
Reference in New Issue
Block a user