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

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;
     }

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_;
 };
 
 /**

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();
   {