__google_stl_debug_vector: erase() at invalid position: folly/experimental/test:futur...

Summary:
```
[ RUN      ] FutureDAGTest.RemoveNodeComplex
terminate called after throwing an instance of 'std::out_of_range'
  what():  erase() at invalid position
*** Aborted at 1470867850 (Unix time, try 'date -d 1470867850') ***
*** Signal 6 (SIGABRT) (0x133c001ed107) received by PID 2019591 (pthread TID 0x7ff1e6df5980) (linux TID 2019591) (maybe from PID 2019591, UID 4924), stack trace: ***
    @ 00007ff1e702020f folly::symbolizer::(anonymous namespace)::innerSignalHandler(int, siginfo_t*, void*)
    @ 00007ff1e701faf1 folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
    @ 00007ff1e87c714f (unknown)
    @ 00007ff1e7cf6d02 gsignal
    @ 00007ff1e7cf8af5 abort
    @ 00007ff1e73880e4 __gnu_cxx::__verbose_terminate_handler()
    @ 00007ff1e7385ee5 __cxxabiv1::__terminate(void (*)())
    @ 00007ff1e7385f30 std::terminate()
    @ 00007ff1e73861c0 __cxa_throw
    @ 00007ff1e740ae00 std::__throw_out_of_range(char const*)
    @ 00000000004483db std::vector<folly::FutureDAG::Node, std::allocator<folly::FutureDAG::Node> >::_M_erase(__gnu_cxx::__normal_iterator<folly::FutureDAG::Node*, std::vector<folly::FutureDAG::Node, std::allocator<folly::FutureDAG::Node> > >)
    @ 00000000004478fa std::vector<folly::FutureDAG::Node, std::allocator<folly::FutureDAG::Node> >::erase(__gnu_cxx::__normal_iterator<folly::FutureDAG::Node const*, std::vector<folly::FutureDAG::Node, std::allocator<folly::FutureDAG::Node> > >)
    @ 000000000044770a folly::FutureDAG::remove(unsigned long)
    @ 0000000000424bc2 FutureDAGTest::remove(unsigned long)
    @ 000000000041f7ed FutureDAGTest_RemoveNodeComplex_Test::TestBody()
    @ 00007ff1e9b19361 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*)
                       /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2420
                       -> /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest-all.cc
    @ 00007ff1e9b029de testing::Test::Run()
                       /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2437
                       -> /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest-all.cc
    @ 00007ff1e9b02aca testing::TestInfo::Run()
                       /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2612
                       -> /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest-all.cc
    @ 00007ff1e9b02cfc testing::TestCase::Run()
                       /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2730
                       -> /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest-all.cc
    @ 00007ff1e9b0d2a7 testing::internal::UnitTestImpl::RunAllTests()
                       /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:4602
                       -> /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest-all.cc
    @ 00007ff1e9b0d60c testing::UnitTest::Run()
                       /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest.cc:2420
                       -> /home/engshare/third-party2/gtest/1.7.0/src/gtest-1.7.0/src/gtest-all.cc
    @ 00007ff1eb64ec70 RUN_ALL_TESTS()
    @ 00007ff1eb64ec3c main
    @ 00007ff1e7ce10f5 __libc_start_main
    @ 000000000041f2d2 (unknown)
Summary (total time 0.07s):
  PASS: 0
  FAIL: 0
  SKIP: 0
  FATAL: 1
    folly/experimental/test:future_dag_test - FutureDAGTest.RemoveNodeComplex
  TIMEOUT: 0
````

Reviewed By: yfeldblum

Differential Revision: D3700956

fbshipit-source-id: dffca9460ccca1d8069218c5b7310e4d6e061697

diff --git a/folly/experimental/FutureDAG.h b/folly/experimental/FutureDAG.h
index 42391336eccfef95a189df69b25cb206c8236c59..3f08fff04d11b645695c8ca19c9d2dad9b465479 100644 (file)
--- a/folly/experimental/FutureDAG.h
+++ b/folly/experimental/FutureDAG.h
@@ -35,7 +35,11 @@ class FutureDAG : public std::enable_shared_from_this<FutureDAG> {
   }
 
   void remove(Handle a) {
-    if (nodes.size() > a && nodes[a].hasDependents) {
+    if (a >= nodes.size()) {
+      return;
+    }
+
+    if (nodes[a].hasDependents) {
       for (auto& node : nodes) {
         auto& deps = node.dependencies;
         deps.erase(
@@ -47,6 +51,7 @@ class FutureDAG : public std::enable_shared_from_this<FutureDAG> {
         }
       }
     }
+
     nodes.erase(nodes.begin() + a);
   }
 

diff --git a/folly/experimental/test/FutureDAGTest.cpp b/folly/experimental/test/FutureDAGTest.cpp
index fd17ae6ed4c1f68487e2d5d8b68542c05f82734c..1fb947aa86aee35d8da217f1adf478c6d4cc1f6e 100644 (file)
--- a/folly/experimental/test/FutureDAGTest.cpp
+++ b/folly/experimental/test/FutureDAGTest.cpp
@@ -53,17 +53,15 @@ struct FutureDAGTest : public testing::Test {
   }
 
   void remove(Handle a) {
-    for (auto itr = nodes.begin(); itr != nodes.end(); itr++) {
-      auto& deps = itr->second->dependencies;
-      if (std::find(deps.begin(), deps.end(), a) != deps.end()) {
-        deps.erase(deps.begin() + a);
-      }
+    for (auto& node : nodes) {
+      node.second->dependencies.erase(a);
     }
     nodes.erase(a);
     dag->remove(a);
   }
+
   void dependency(Handle a, Handle b) {
-    nodes.at(b)->dependencies.push_back(a);
+    nodes.at(b)->dependencies.insert(a);
     dag->dependency(a, b);
   }
 
@@ -83,20 +81,19 @@ struct FutureDAGTest : public testing::Test {
   }
 
   struct TestNode {
-    explicit TestNode(FutureDAGTest* test) {
-      func = [this, test] {
-        test->order.push_back(handle);
-        return Future<Unit>();
-      };
-      handle = test->dag->add(func);
-    }
+    explicit TestNode(FutureDAGTest* test)
+        : func([this, test] {
+            test->order.push_back(handle);
+            return Future<Unit>();
+          }),
+          handle(test->dag->add(func)) {}
 
-    FutureDAG::FutureFunc func;
-    Handle handle;
-    std::vector<Handle> dependencies;
+    const FutureDAG::FutureFunc func;
+    const Handle handle;
+    std::set<Handle> dependencies;
   };
 
-  std::shared_ptr<FutureDAG> dag = FutureDAG::create();
+  const std::shared_ptr<FutureDAG> dag = FutureDAG::create();
   std::map<Handle, std::unique_ptr<TestNode>> nodes;
   std::vector<Handle> order;
 };
@@ -121,9 +118,9 @@ TEST_F(FutureDAGTest, RemoveNodeComplex) {
   auto h3 = add();
   dependency(h1, h3);
   dependency(h2, h1);
-  remove(h1);
-  remove(h2);
   remove(h3);
+  remove(h2);
+  remove(h1);
   ASSERT_NO_THROW(dag->go().get());
   checkOrder();
 }