3 files changed, 140 insertions, 127 deletions

diff --git a/src/leveldb/helpers/memenv/memenv.cc b/src/leveldb/helpers/memenv/memenv.cc
index 68c0614a59..47e4481f7c 100644
--- a/src/leveldb/helpers/memenv/memenv.cc
+++ b/src/leveldb/helpers/memenv/memenv.cc
@@ -4,14 +4,18 @@
 
 #include "helpers/memenv/memenv.h"
 
+#include <string.h>
+
+#include <limits>
+#include <map>
+#include <string>
+#include <vector>
+
 #include "leveldb/env.h"
 #include "leveldb/status.h"
 #include "port/port.h"
+#include "port/thread_annotations.h"
 #include "util/mutexlock.h"
-#include <map>
-#include <string.h>
-#include <string>
-#include <vector>
 
 namespace leveldb {
 
@@ -23,6 +27,10 @@ class FileState {
   // and the caller must call Ref() at least once.
   FileState() : refs_(0), size_(0) {}
 
+  // No copying allowed.
+  FileState(const FileState&) = delete;
+  FileState& operator=(const FileState&) = delete;
+
   // Increase the reference count.
   void Ref() {
     MutexLock lock(&refs_mutex_);
@@ -47,9 +55,22 @@ class FileState {
     }
   }
 
-  uint64_t Size() const { return size_; }
+  uint64_t Size() const {
+    MutexLock lock(&blocks_mutex_);
+    return size_;
+  }
+
+  void Truncate() {
+    MutexLock lock(&blocks_mutex_);
+    for (char*& block : blocks_) {
+      delete[] block;
+    }
+    blocks_.clear();
+    size_ = 0;
+  }
 
   Status Read(uint64_t offset, size_t n, Slice* result, char* scratch) const {
+    MutexLock lock(&blocks_mutex_);
     if (offset > size_) {
       return Status::IOError("Offset greater than file size.");
     }
@@ -62,16 +83,9 @@ class FileState {
       return Status::OK();
     }
 
-    assert(offset / kBlockSize <= SIZE_MAX);
+    assert(offset / kBlockSize <= std::numeric_limits<size_t>::max());
     size_t block = static_cast<size_t>(offset / kBlockSize);
     size_t block_offset = offset % kBlockSize;
-
-    if (n <= kBlockSize - block_offset) {
-      // The requested bytes are all in the first block.
-      *result = Slice(blocks_[block] + block_offset, n);
-      return Status::OK();
-    }
-
     size_t bytes_to_copy = n;
     char* dst = scratch;
 
@@ -96,6 +110,7 @@ class FileState {
     const char* src = data.data();
     size_t src_len = data.size();
 
+    MutexLock lock(&blocks_mutex_);
     while (src_len > 0) {
       size_t avail;
       size_t offset = size_ % kBlockSize;
@@ -122,28 +137,17 @@ class FileState {
   }
 
  private:
-  // Private since only Unref() should be used to delete it.
-  ~FileState() {
-    for (std::vector<char*>::iterator i = blocks_.begin(); i != blocks_.end();
-         ++i) {
-      delete [] *i;
-    }
-  }
+  enum { kBlockSize = 8 * 1024 };
 
-  // No copying allowed.
-  FileState(const FileState&);
-  void operator=(const FileState&);
+  // Private since only Unref() should be used to delete it.
+  ~FileState() { Truncate(); }
 
   port::Mutex refs_mutex_;
-  int refs_;  // Protected by refs_mutex_;
+  int refs_ GUARDED_BY(refs_mutex_);
 
-  // The following fields are not protected by any mutex. They are only mutable
-  // while the file is being written, and concurrent access is not allowed
-  // to writable files.
-  std::vector<char*> blocks_;
-  uint64_t size_;
-
-  enum { kBlockSize = 8 * 1024 };
+  mutable port::Mutex blocks_mutex_;
+  std::vector<char*> blocks_ GUARDED_BY(blocks_mutex_);
+  uint64_t size_ GUARDED_BY(blocks_mutex_);
 };
 
 class SequentialFileImpl : public SequentialFile {
@@ -152,11 +156,9 @@ class SequentialFileImpl : public SequentialFile {
     file_->Ref();
   }
 
-  ~SequentialFileImpl() {
-    file_->Unref();
-  }
+  ~SequentialFileImpl() override { file_->Unref(); }
 
-  virtual Status Read(size_t n, Slice* result, char* scratch) {
+  Status Read(size_t n, Slice* result, char* scratch) override {
     Status s = file_->Read(pos_, n, result, scratch);
     if (s.ok()) {
       pos_ += result->size();
@@ -164,7 +166,7 @@ class SequentialFileImpl : public SequentialFile {
     return s;
   }
 
-  virtual Status Skip(uint64_t n) {
+  Status Skip(uint64_t n) override {
     if (pos_ > file_->Size()) {
       return Status::IOError("pos_ > file_->Size()");
     }
@@ -176,7 +178,7 @@ class SequentialFileImpl : public SequentialFile {
     return Status::OK();
   }
 
-  virtual std::string GetName() const { return "[memenv]"; }
+  virtual std::string GetName() const override { return "[memenv]"; }
  private:
   FileState* file_;
   uint64_t pos_;
@@ -184,68 +186,58 @@ class SequentialFileImpl : public SequentialFile {
 
 class RandomAccessFileImpl : public RandomAccessFile {
  public:
-  explicit RandomAccessFileImpl(FileState* file) : file_(file) {
-    file_->Ref();
-  }
+  explicit RandomAccessFileImpl(FileState* file) : file_(file) { file_->Ref(); }
 
-  ~RandomAccessFileImpl() {
-    file_->Unref();
-  }
+  ~RandomAccessFileImpl() override { file_->Unref(); }
 
-  virtual Status Read(uint64_t offset, size_t n, Slice* result,
-                      char* scratch) const {
+  Status Read(uint64_t offset, size_t n, Slice* result,
+              char* scratch) const override {
     return file_->Read(offset, n, result, scratch);
   }
 
-  virtual std::string GetName() const { return "[memenv]"; }
+  virtual std::string GetName() const override { return "[memenv]"; }
  private:
   FileState* file_;
 };
 
 class WritableFileImpl : public WritableFile {
  public:
-  WritableFileImpl(FileState* file) : file_(file) {
-    file_->Ref();
-  }
+  WritableFileImpl(FileState* file) : file_(file) { file_->Ref(); }
 
-  ~WritableFileImpl() {
-    file_->Unref();
-  }
+  ~WritableFileImpl() override { file_->Unref(); }
 
-  virtual Status Append(const Slice& data) {
-    return file_->Append(data);
-  }
+  Status Append(const Slice& data) override { return file_->Append(data); }
 
-  virtual Status Close() { return Status::OK(); }
-  virtual Status Flush() { return Status::OK(); }
-  virtual Status Sync() { return Status::OK(); }
+  Status Close() override { return Status::OK(); }
+  Status Flush() override { return Status::OK(); }
+  Status Sync() override { return Status::OK(); }
 
-  virtual std::string GetName() const { return "[memenv]"; }
+  virtual std::string GetName() const override { return "[memenv]"; }
  private:
   FileState* file_;
 };
 
 class NoOpLogger : public Logger {
  public:
-  virtual void Logv(const char* format, va_list ap) { }
+  void Logv(const char* format, va_list ap) override {}
 };
 
 class InMemoryEnv : public EnvWrapper {
  public:
-  explicit InMemoryEnv(Env* base_env) : EnvWrapper(base_env) { }
+  explicit InMemoryEnv(Env* base_env) : EnvWrapper(base_env) {}
 
-  virtual ~InMemoryEnv() {
-    for (FileSystem::iterator i = file_map_.begin(); i != file_map_.end(); ++i){
-      i->second->Unref();
+  ~InMemoryEnv() override {
+    for (const auto& kvp : file_map_) {
+      kvp.second->Unref();
     }
   }
 
   // Partial implementation of the Env interface.
-  virtual Status NewSequentialFile(const std::string& fname,
-                                   SequentialFile** result) {
+  Status NewSequentialFile(const std::string& fname,
+                           SequentialFile** result) override {
     MutexLock lock(&mutex_);
     if (file_map_.find(fname) == file_map_.end()) {
-      *result = NULL;
+      *result = nullptr;
       return Status::IOError(fname, "File not found");
     }
 
@@ -253,11 +245,11 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  virtual Status NewRandomAccessFile(const std::string& fname,
-                                     RandomAccessFile** result) {
+  Status NewRandomAccessFile(const std::string& fname,
+                             RandomAccessFile** result) override {
     MutexLock lock(&mutex_);
     if (file_map_.find(fname) == file_map_.end()) {
-      *result = NULL;
+      *result = nullptr;
       return Status::IOError(fname, "File not found");
     }
 
@@ -265,27 +257,32 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  virtual Status NewWritableFile(const std::string& fname,
-                                 WritableFile** result) {
+  Status NewWritableFile(const std::string& fname,
+                         WritableFile** result) override {
     MutexLock lock(&mutex_);
-    if (file_map_.find(fname) != file_map_.end()) {
-      DeleteFileInternal(fname);
-    }
+    FileSystem::iterator it = file_map_.find(fname);
 
-    FileState* file = new FileState();
-    file->Ref();
-    file_map_[fname] = file;
+    FileState* file;
+    if (it == file_map_.end()) {
+      // File is not currently open.
+      file = new FileState();
+      file->Ref();
+      file_map_[fname] = file;
+    } else {
+      file = it->second;
+      file->Truncate();
+    }
 
     *result = new WritableFileImpl(file);
     return Status::OK();
   }
 
-  virtual Status NewAppendableFile(const std::string& fname,
-                                   WritableFile** result) {
+  Status NewAppendableFile(const std::string& fname,
+                           WritableFile** result) override {
     MutexLock lock(&mutex_);
     FileState** sptr = &file_map_[fname];
     FileState* file = *sptr;
-    if (file == NULL) {
+    if (file == nullptr) {
       file = new FileState();
       file->Ref();
     }
@@ -293,18 +290,18 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  virtual bool FileExists(const std::string& fname) {
+  bool FileExists(const std::string& fname) override {
     MutexLock lock(&mutex_);
     return file_map_.find(fname) != file_map_.end();
   }
 
-  virtual Status GetChildren(const std::string& dir,
-                             std::vector<std::string>* result) {
+  Status GetChildren(const std::string& dir,
+                     std::vector<std::string>* result) override {
     MutexLock lock(&mutex_);
     result->clear();
 
-    for (FileSystem::iterator i = file_map_.begin(); i != file_map_.end(); ++i){
-      const std::string& filename = i->first;
+    for (const auto& kvp : file_map_) {
+      const std::string& filename = kvp.first;
 
       if (filename.size() >= dir.size() + 1 && filename[dir.size()] == '/' &&
           Slice(filename).starts_with(Slice(dir))) {
@@ -315,7 +312,8 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  void DeleteFileInternal(const std::string& fname) {
+  void DeleteFileInternal(const std::string& fname)
+      EXCLUSIVE_LOCKS_REQUIRED(mutex_) {
     if (file_map_.find(fname) == file_map_.end()) {
       return;
     }
@@ -324,7 +322,7 @@ class InMemoryEnv : public EnvWrapper {
     file_map_.erase(fname);
   }
 
-  virtual Status DeleteFile(const std::string& fname) {
+  Status DeleteFile(const std::string& fname) override {
     MutexLock lock(&mutex_);
     if (file_map_.find(fname) == file_map_.end()) {
       return Status::IOError(fname, "File not found");
@@ -334,15 +332,11 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  virtual Status CreateDir(const std::string& dirname) {
-    return Status::OK();
-  }
+  Status CreateDir(const std::string& dirname) override { return Status::OK(); }
 
-  virtual Status DeleteDir(const std::string& dirname) {
-    return Status::OK();
-  }
+  Status DeleteDir(const std::string& dirname) override { return Status::OK(); }
 
-  virtual Status GetFileSize(const std::string& fname, uint64_t* file_size) {
+  Status GetFileSize(const std::string& fname, uint64_t* file_size) override {
     MutexLock lock(&mutex_);
     if (file_map_.find(fname) == file_map_.end()) {
       return Status::IOError(fname, "File not found");
@@ -352,8 +346,8 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  virtual Status RenameFile(const std::string& src,
-                            const std::string& target) {
+  Status RenameFile(const std::string& src,
+                    const std::string& target) override {
     MutexLock lock(&mutex_);
     if (file_map_.find(src) == file_map_.end()) {
       return Status::IOError(src, "File not found");
@@ -365,22 +359,22 @@ class InMemoryEnv : public EnvWrapper {
     return Status::OK();
   }
 
-  virtual Status LockFile(const std::string& fname, FileLock** lock) {
+  Status LockFile(const std::string& fname, FileLock** lock) override {
     *lock = new FileLock;
     return Status::OK();
   }
 
-  virtual Status UnlockFile(FileLock* lock) {
+  Status UnlockFile(FileLock* lock) override {
     delete lock;
     return Status::OK();
   }
 
-  virtual Status GetTestDirectory(std::string* path) {
+  Status GetTestDirectory(std::string* path) override {
     *path = "/test";
     return Status::OK();
   }
 
-  virtual Status NewLogger(const std::string& fname, Logger** result) {
+  Status NewLogger(const std::string& fname, Logger** result) override {
     *result = new NoOpLogger;
     return Status::OK();
   }
@@ -388,14 +382,13 @@ class InMemoryEnv : public EnvWrapper {
  private:
   // Map from filenames to FileState objects, representing a simple file system.
   typedef std::map<std::string, FileState*> FileSystem;
+
   port::Mutex mutex_;
-  FileSystem file_map_;  // Protected by mutex_.
+  FileSystem file_map_ GUARDED_BY(mutex_);
 };
 
 }  // namespace
 
-Env* NewMemEnv(Env* base_env) {
-  return new InMemoryEnv(base_env);
-}
+Env* NewMemEnv(Env* base_env) { return new InMemoryEnv(base_env); }
 
 }  // namespace leveldb
diff --git a/src/leveldb/helpers/memenv/memenv.h b/src/leveldb/helpers/memenv/memenv.h
index 03b88de761..3d929e4c4e 100644
--- a/src/leveldb/helpers/memenv/memenv.h
+++ b/src/leveldb/helpers/memenv/memenv.h
@@ -5,6 +5,8 @@
 #ifndef STORAGE_LEVELDB_HELPERS_MEMENV_MEMENV_H_
 #define STORAGE_LEVELDB_HELPERS_MEMENV_MEMENV_H_
 
+#include "leveldb/export.h"
+
 namespace leveldb {
 
 class Env;
@@ -13,7 +15,7 @@ class Env;
 // all non-file-storage tasks to base_env. The caller must delete the result
 // when it is no longer needed.
 // *base_env must remain live while the result is in use.
-Env* NewMemEnv(Env* base_env);
+LEVELDB_EXPORT Env* NewMemEnv(Env* base_env);
 
 }  // namespace leveldb
 
diff --git a/src/leveldb/helpers/memenv/memenv_test.cc b/src/leveldb/helpers/memenv/memenv_test.cc
index 5cff77613f..94ad06be68 100644
--- a/src/leveldb/helpers/memenv/memenv_test.cc
+++ b/src/leveldb/helpers/memenv/memenv_test.cc
@@ -4,25 +4,22 @@
 
 #include "helpers/memenv/memenv.h"
 
+#include <string>
+#include <vector>
+
 #include "db/db_impl.h"
 #include "leveldb/db.h"
 #include "leveldb/env.h"
 #include "util/testharness.h"
-#include <string>
-#include <vector>
 
 namespace leveldb {
 
 class MemEnvTest {
  public:
-  Env* env_;
+  MemEnvTest() : env_(NewMemEnv(Env::Default())) {}
+  ~MemEnvTest() { delete env_; }
 
-  MemEnvTest()
-      : env_(NewMemEnv(Env::Default())) {
-  }
-  ~MemEnvTest() {
-    delete env_;
-  }
+  Env* env_;
 };
 
 TEST(MemEnvTest, Basics) {
@@ -109,25 +106,25 @@ TEST(MemEnvTest, ReadWrite) {
 
   // Read sequentially.
   ASSERT_OK(env_->NewSequentialFile("/dir/f", &seq_file));
-  ASSERT_OK(seq_file->Read(5, &result, scratch)); // Read "hello".
+  ASSERT_OK(seq_file->Read(5, &result, scratch));  // Read "hello".
   ASSERT_EQ(0, result.compare("hello"));
   ASSERT_OK(seq_file->Skip(1));
-  ASSERT_OK(seq_file->Read(1000, &result, scratch)); // Read "world".
+  ASSERT_OK(seq_file->Read(1000, &result, scratch));  // Read "world".
   ASSERT_EQ(0, result.compare("world"));
-  ASSERT_OK(seq_file->Read(1000, &result, scratch)); // Try reading past EOF.
+  ASSERT_OK(seq_file->Read(1000, &result, scratch));  // Try reading past EOF.
   ASSERT_EQ(0, result.size());
-  ASSERT_OK(seq_file->Skip(100)); // Try to skip past end of file.
+  ASSERT_OK(seq_file->Skip(100));  // Try to skip past end of file.
   ASSERT_OK(seq_file->Read(1000, &result, scratch));
   ASSERT_EQ(0, result.size());
   delete seq_file;
 
   // Random reads.
   ASSERT_OK(env_->NewRandomAccessFile("/dir/f", &rand_file));
-  ASSERT_OK(rand_file->Read(6, 5, &result, scratch)); // Read "world".
+  ASSERT_OK(rand_file->Read(6, 5, &result, scratch));  // Read "world".
   ASSERT_EQ(0, result.compare("world"));
-  ASSERT_OK(rand_file->Read(0, 5, &result, scratch)); // Read "hello".
+  ASSERT_OK(rand_file->Read(0, 5, &result, scratch));  // Read "hello".
   ASSERT_EQ(0, result.compare("hello"));
-  ASSERT_OK(rand_file->Read(10, 100, &result, scratch)); // Read "d".
+  ASSERT_OK(rand_file->Read(10, 100, &result, scratch));  // Read "d".
   ASSERT_EQ(0, result.compare("d"));
 
   // Too high offset.
@@ -176,7 +173,7 @@ TEST(MemEnvTest, LargeWrite) {
   SequentialFile* seq_file;
   Slice result;
   ASSERT_OK(env_->NewSequentialFile("/dir/f", &seq_file));
-  ASSERT_OK(seq_file->Read(3, &result, scratch)); // Read "foo".
+  ASSERT_OK(seq_file->Read(3, &result, scratch));  // Read "foo".
   ASSERT_EQ(0, result.compare("foo"));
 
   size_t read = 0;
@@ -188,7 +185,30 @@ TEST(MemEnvTest, LargeWrite) {
   }
   ASSERT_TRUE(write_data == read_data);
   delete seq_file;
-  delete [] scratch;
+  delete[] scratch;
+}
+
+TEST(MemEnvTest, OverwriteOpenFile) {
+  const char kWrite1Data[] = "Write #1 data";
+  const size_t kFileDataLen = sizeof(kWrite1Data) - 1;
+  const std::string kTestFileName = test::TmpDir() + "/leveldb-TestFile.dat";
+
+  ASSERT_OK(WriteStringToFile(env_, kWrite1Data, kTestFileName));
+
+  RandomAccessFile* rand_file;
+  ASSERT_OK(env_->NewRandomAccessFile(kTestFileName, &rand_file));
+
+  const char kWrite2Data[] = "Write #2 data";
+  ASSERT_OK(WriteStringToFile(env_, kWrite2Data, kTestFileName));
+
+  // Verify that overwriting an open file will result in the new file data
+  // being read from files opened before the write.
+  Slice result;
+  char scratch[kFileDataLen];
+  ASSERT_OK(rand_file->Read(0, kFileDataLen, &result, scratch));
+  ASSERT_EQ(0, result.compare(kWrite2Data));
+
+  delete rand_file;
 }
 
 TEST(MemEnvTest, DBTest) {
@@ -236,6 +256,4 @@ TEST(MemEnvTest, DBTest) {
 
 }  // namespace leveldb
 
-int main(int argc, char** argv) {
-  return leveldb::test::RunAllTests();
-}
+int main(int argc, char** argv) { return leveldb::test::RunAllTests(); }