From 4985f60fc88f6bd8e715067ef02d443dea32a826 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Thu, 6 Oct 2016 12:43:05 -0500 Subject: [PATCH] env_mirror: fix a few leaks (#1363) * env_mirror: fix leak from LockFile Signed-off-by: Sage Weil * env_mirror: instruct EnvMirror whether mirrored Envs should be destroyed The lifecycle rules for Env are frustrating and undocumented. Notably, Env::Default() should *not* be freed, but any Env instances we created should be. Explicitly instruct EnvMirror whether to clean up child Env instances. Default to false so that we do not affect existing callers. Signed-off-by: Sage Weil --- include/rocksdb/utilities/env_mirror.h | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/include/rocksdb/utilities/env_mirror.h b/include/rocksdb/utilities/env_mirror.h index 021fbfa45..091c92a0e 100644 --- a/include/rocksdb/utilities/env_mirror.h +++ b/include/rocksdb/utilities/env_mirror.h @@ -33,9 +33,21 @@ class WritableFileMirror; class EnvMirror : public EnvWrapper { Env* a_, *b_; + bool free_a_, free_b_; public: - EnvMirror(Env* a, Env* b) : EnvWrapper(a), a_(a), b_(b) {} + EnvMirror(Env* a, Env* b, bool free_a=false, bool free_b=false) + : EnvWrapper(a), + a_(a), + b_(b), + free_a_(free_a), + free_b_(free_b) {} + ~EnvMirror() { + if (free_a_) + delete a_; + if (free_b_) + delete b_; + } Status NewSequentialFile(const std::string& f, unique_ptr* r, const EnvOptions& options) override; @@ -155,6 +167,7 @@ class EnvMirror : public EnvWrapper { Status as = a_->UnlockFile(ml->a_); Status bs = b_->UnlockFile(ml->b_); assert(as == bs); + delete ml; return as; } };