From: Joseph Griego <jgriego@fb.com>
Date: Tue, 24 May 2016 20:14:54 +0000 (-0700)
Subject: Clear old fs::path when moving a TemporaryDirectory
X-Git-Tag: 2016.07.26~207
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=e6edf05a6ff6b4729f599fbc4aabece2fb7871cf;p=folly.git

Clear old fs::path when moving a TemporaryDirectory

Summary:
If a TemporaryDirectory with scope DELETE_ON_DESTRUCTION gets moved
and then goes out of scope, the directory will be deleted while the moved
object still holds that path; we just clear the path from the old object to
prevent this

Reviewed By: yfeldblum

Differential Revision: D3331079

fbshipit-source-id: 9c99413661e2436ccec937d3fa652da810133b34
---

diff --git a/folly/experimental/TestUtil.cpp b/folly/experimental/TestUtil.cpp
index e1f2010b..362ed158 100644
--- a/folly/experimental/TestUtil.cpp
+++ b/folly/experimental/TestUtil.cpp
@@ -95,18 +95,20 @@ TemporaryFile::~TemporaryFile() {
   }
 }
 
-TemporaryDirectory::TemporaryDirectory(StringPiece namePrefix,
-                                       fs::path dir,
-                                       Scope scope)
-  : scope_(scope),
-    path_(generateUniquePath(std::move(dir), namePrefix)) {
-  fs::create_directory(path_);
+TemporaryDirectory::TemporaryDirectory(
+    StringPiece namePrefix,
+    fs::path dir,
+    Scope scope)
+    : scope_(scope),
+      path_(std::make_unique<fs::path>(
+          generateUniquePath(std::move(dir), namePrefix))) {
+  fs::create_directory(path());
 }
 
 TemporaryDirectory::~TemporaryDirectory() {
-  if (scope_ == Scope::DELETE_ON_DESTRUCTION) {
+  if (scope_ == Scope::DELETE_ON_DESTRUCTION && path_ != nullptr) {
     boost::system::error_code ec;
-    fs::remove_all(path_, ec);
+    fs::remove_all(path(), ec);
     if (ec) {
       LOG(WARNING) << "recursive delete on destruction failed: " << ec;
     }
diff --git a/folly/experimental/TestUtil.h b/folly/experimental/TestUtil.h
index f5fb1638..5b4112fe 100644
--- a/folly/experimental/TestUtil.h
+++ b/folly/experimental/TestUtil.h
@@ -89,11 +89,13 @@ class TemporaryDirectory {
   TemporaryDirectory(TemporaryDirectory&&) = default;
   TemporaryDirectory& operator=(TemporaryDirectory&&) = default;
 
-  const fs::path& path() const { return path_; }
+  const fs::path& path() const {
+    return *path_;
+  }
 
  private:
   Scope scope_;
-  fs::path path_;
+  std::unique_ptr<fs::path> path_;
 };
 
 /**
diff --git a/folly/experimental/test/TestUtilTest.cpp b/folly/experimental/test/TestUtilTest.cpp
index 8e69da9c..d8480b1d 100644
--- a/folly/experimental/test/TestUtilTest.cpp
+++ b/folly/experimental/test/TestUtilTest.cpp
@@ -101,6 +101,30 @@ TEST(TemporaryDirectory, DeleteOnDestruction) {
   testTemporaryDirectory(TemporaryDirectory::Scope::DELETE_ON_DESTRUCTION);
 }
 
+void expectTempdirExists(const TemporaryDirectory& d) {
+  EXPECT_FALSE(d.path().empty());
+  EXPECT_TRUE(fs::exists(d.path()));
+  EXPECT_TRUE(fs::is_directory(d.path()));
+}
+
+TEST(TemporaryDirectory, SafelyMove) {
+  std::unique_ptr<TemporaryDirectory> dir;
+  TemporaryDirectory dir2;
+  {
+    auto scope = TemporaryDirectory::Scope::DELETE_ON_DESTRUCTION;
+    TemporaryDirectory d("", "", scope);
+    TemporaryDirectory d2("", "", scope);
+    expectTempdirExists(d);
+    expectTempdirExists(d2);
+
+    dir = std::make_unique<TemporaryDirectory>(std::move(d));
+    dir2 = std::move(d2);
+  }
+
+  expectTempdirExists(*dir);
+  expectTempdirExists(dir2);
+}
+
 TEST(ChangeToTempDir, ChangeDir) {
   auto pwd1 = fs::current_path();
   {