Fix/improve 'must free heap allocations' code (#9209)

Summary:
Added missing include, and cleaned up to make same mistake less
likely in future (minimize conditional compilation)

Fixes https://github.com/facebook/rocksdb/issues/9183

Pull Request resolved: https://github.com/facebook/rocksdb/pull/9209

Test Plan: added to existing test

Reviewed By: mrambacher

Differential Revision: D32631390

Pulled By: pdillinger

fbshipit-source-id: 63a0501855cf5fac9e22ca1e5c4f53725dbf3f93
This commit is contained in:
Peter Dillinger 2021-11-29 10:52:32 -08:00 committed by Facebook GitHub Bot
parent 735fe61e8f
commit fc3a6eb74a
4 changed files with 17 additions and 10 deletions

3
cache/cache_test.cc vendored
View File

@ -609,6 +609,9 @@ TEST_P(CacheTest, SetCapacity) {
for (size_t i = 5; i < 10; i++) { for (size_t i = 5; i < 10; i++) {
cache->Release(handles[i]); cache->Release(handles[i]);
} }
// Make sure this doesn't crash or upset ASAN/valgrind
cache->DisownData();
} }
TEST_P(LRUCacheTest, SetStrictCapacityLimit) { TEST_P(LRUCacheTest, SetStrictCapacityLimit) {

11
cache/clock_cache.cc vendored
View File

@ -33,11 +33,11 @@ std::shared_ptr<Cache> NewClockCache(
#ifndef ROCKSDB_USE_RTTI #ifndef ROCKSDB_USE_RTTI
#define TBB_USE_EXCEPTIONS 0 #define TBB_USE_EXCEPTIONS 0
#endif #endif
#include "tbb/concurrent_hash_map.h"
#include "cache/sharded_cache.h" #include "cache/sharded_cache.h"
#include "port/lang.h"
#include "port/malloc.h" #include "port/malloc.h"
#include "port/port.h" #include "port/port.h"
#include "tbb/concurrent_hash_map.h"
#include "util/autovector.h" #include "util/autovector.h"
#include "util/mutexlock.h" #include "util/mutexlock.h"
@ -809,9 +809,10 @@ class ClockCache final : public ShardedCache {
} }
void DisownData() override { void DisownData() override {
#ifndef MUST_FREE_HEAP_ALLOCATIONS // Leak data only if that won't generate an ASAN/valgrind warning
shards_ = nullptr; if (!kMustFreeHeapAllocations) {
#endif shards_ = nullptr;
}
} }
void WaitAll(std::vector<Handle*>& /*handles*/) override {} void WaitAll(std::vector<Handle*>& /*handles*/) override {}

11
cache/lru_cache.cc vendored
View File

@ -15,6 +15,7 @@
#include "monitoring/perf_context_imp.h" #include "monitoring/perf_context_imp.h"
#include "monitoring/statistics.h" #include "monitoring/statistics.h"
#include "port/lang.h"
#include "util/mutexlock.h" #include "util/mutexlock.h"
namespace ROCKSDB_NAMESPACE { namespace ROCKSDB_NAMESPACE {
@ -704,11 +705,11 @@ uint32_t LRUCache::GetHash(Handle* handle) const {
} }
void LRUCache::DisownData() { void LRUCache::DisownData() {
// Do not drop data if compile with ASAN to suppress leak warning. // Leak data only if that won't generate an ASAN/valgrind warning
#ifndef MUST_FREE_HEAP_ALLOCATIONS if (!kMustFreeHeapAllocations) {
shards_ = nullptr; shards_ = nullptr;
num_shards_ = 0; num_shards_ = 0;
#endif }
} }
size_t LRUCache::TEST_GetLRUSize() { size_t LRUCache::TEST_GetLRUSize() {

View File

@ -42,8 +42,10 @@
// STATIC_AVOID_DESTRUCTION(Foo, foo)(arg1, arg2); // STATIC_AVOID_DESTRUCTION(Foo, foo)(arg1, arg2);
#ifdef MUST_FREE_HEAP_ALLOCATIONS #ifdef MUST_FREE_HEAP_ALLOCATIONS
#define STATIC_AVOID_DESTRUCTION(Type, name) static Type name #define STATIC_AVOID_DESTRUCTION(Type, name) static Type name
constexpr bool kMustFreeHeapAllocations = true;
#else #else
#define STATIC_AVOID_DESTRUCTION(Type, name) static Type& name = *new Type #define STATIC_AVOID_DESTRUCTION(Type, name) static Type& name = *new Type
constexpr bool kMustFreeHeapAllocations = false;
#endif #endif
// TSAN (Thread sanitizer) // TSAN (Thread sanitizer)