Skip to content

Commit b5bd466

Browse files
committed
rgw/driver/posix: closedir() to free dir handle
Previously, we had Resource leak in Directory::for_each() where directory streams opened with fdopendir() were never properly closed, causing a 32KB leak per unclosed directory handle. In this change, we add closedir() call to properly release directory stream resources after enumeration completes. ASan report: ``` Direct leak of 32816 byte(s) in 1 object(s) allocated from: #0 0x738387320e15 in malloc /usr/src/debug/gcc/gcc/libsanitizer/asan/asan_malloc_linux.cpp:67 #1 0x738381383514 (/usr/lib/libc.so.6+0xe3514) (BuildId: 468e3585c794491a48ea75fceb9e4d6b1464fc35) #2 0x738381383418 in fdopendir (/usr/lib/libc.so.6+0xe3418) (BuildId: 468e3585c794491a48ea75fceb9e4d6b1464fc35) ceph#3 0x6433e1aefac4 in for_each<rgw::sal::Directory::fill_cache(const DoutPrefixProvider*, optional_yield, rgw::sal::fill_cache_cb_t&)::<lambda(char const*)> > /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:874 ceph#4 0x6433e1aa10ab in rgw::sal::Directory::fill_cache(DoutPrefixProvider const*, optional_yield, fu2::abi_310::detail::function<fu2::abi_310::detail::config<true, false, 16ul>, fu2::abi_310::detail::property<true, false, int (DoutPrefixProvider const*, rgw_bucket_dir_ent ry&) const> > const&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:1077 ceph#5 0x6433e1aa8974 in rgw::sal::MPDirectory::fill_cache(DoutPrefixProvider const*, optional_yield, fu2::abi_310::detail::function<fu2::abi_310::detail::config<true, false, 16ul>, fu2::abi_310::detail::property<true, false, int (DoutPrefixProvider const*, rgw_bucket_dir_e ntry&) const> > const&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:1384 ceph#6 0x6433e1abf1ee in rgw::sal::POSIXBucket::fill_cache(DoutPrefixProvider const*, optional_yield, fu2::abi_310::detail::function<fu2::abi_310::detail::config<true, false, 16ul>, fu2::abi_310::detail::property<true, false, int (DoutPrefixProvider const*, rgw_bucket_dir_e ntry&) const> > const&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:2236 ceph#7 0x6433e1c43956 in file::listing::BucketCache<rgw::sal::POSIXDriver, rgw::sal::POSIXBucket>::fill(DoutPrefixProvider const*, file::listing::BucketCacheEntry<rgw::sal::POSIXDriver, rgw::sal::POSIXBucket>*, rgw::sal::POSIXBucket*, unsigned int, optional_yield) /home/kef u/dev/ceph/src/rgw/driver/posix/bucket_cache.h:368 ceph#8 0x6433e1c1c71c in file::listing::BucketCache<rgw::sal::POSIXDriver, rgw::sal::POSIXBucket>::list_bucket(DoutPrefixProvider const*, optional_yield, rgw::sal::POSIXBucket*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, fu2::abi_310::d etail::function<fu2::abi_310::detail::config<true, false, 16ul>, fu2::abi_310::detail::property<true, false, bool (rgw_bucket_dir_entry const&) const> >) /home/kefu/dev/ceph/src/rgw/driver/posix/bucket_cache.h:410 ceph#9 0x6433e1ac1829 in rgw::sal::POSIXBucket::list(DoutPrefixProvider const*, rgw::sal::Bucket::ListParams&, int, rgw::sal::Bucket::ListResults&, optional_yield) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:2256 ceph#10 0x6433e1ae1302 in rgw::sal::POSIXMultipartUpload::list_parts(DoutPrefixProvider const*, ceph::common::CephContext*, int, int, int*, bool*, optional_yield, bool) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:3692 ceph#11 0x6433e1ae2f3b in rgw::sal::POSIXMultipartUpload::complete(DoutPrefixProvider const*, optional_yield, ceph::common::CephContext*, std::map<int, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<int>, std::allocator<std::pair< int const, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > >&, std::__cxx11::list<rgw_obj_index_key, std::allocator<rgw_obj_index_key> >&, unsigned long&, bool&, RGWCompressionInfo&, long&, std::__cxx11::basic_string<char, std::char_trait s<char>, std::allocator<char> >&, ACLOwner&, unsigned long, rgw::sal::Object*, boost::container::flat_map<unsigned int, boost::container::flat_set<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::less<std::__cxx11::basic_string<char, std ::char_traits<char>, std::allocator<char> > >, void>, std::less<unsigned int>, void>&) /home/kefu/dev/ceph/src/rgw/driver/posix/rgw_sal_posix.cc:3770 ceph#12 0x6433e0fffb73 in POSIXMPObjectTest::create_MPObj(rgw::sal::MultipartUpload*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >) /home/kefu/dev/ceph/src/test/rgw/test_rgw_posix_driver.cc:1717 ceph#13 0x6433e0e95ab9 in POSIXMPObjectTest_BucketList_Test::TestBody() /home/kefu/dev/ceph/src/test/rgw/test_rgw_posix_driver.cc:1863 ceph#14 0x6433e1308d17 in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) /home/kefu/dev/ceph/src/googletest/googletest/src/gtest.cc:2653 ``` Fixes https://tracker.ceph.com/issues/71505 Signed-off-by: Kefu Chai <tchaikov@gmail.com>
1 parent 24bc646 commit b5bd466

File tree

1 file changed

+8
-0
lines changed

1 file changed

+8
-0
lines changed

src/rgw/driver/posix/rgw_sal_posix.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,7 @@ static int delete_directory(int parent_fd, const char* dname, bool delete_childr
351351
return -ret;
352352
}
353353
}
354+
closedir(dir);
354355

355356
ret = unlinkat(parent_fd, dname, AT_REMOVEDIR);
356357
if (ret < 0) {
@@ -899,6 +900,13 @@ int Directory::for_each(const DoutPrefixProvider* dpp, const F& func)
899900
/* Limit reached */
900901
ret = 0;
901902
}
903+
904+
closedir(dir);
905+
// closedir() closes the fd, so we need to invalidate it
906+
fd = -1;
907+
// closedir() closes fd, but succeeding calls might assume that fd is still valid.
908+
// so let's reopen it.
909+
open(dpp);
902910
return ret;
903911
}
904912

0 commit comments

Comments
 (0)