Fix some folly memory leaks found with clang:dev + asan

Summary:
Reverts the revert of the small_vector and test parts of D1569246.
I'm saving the SocketAddress fix for later, because it caused problems
and needs more research.

Test Plan:
unit tests with clang:dev/asan

Reviewed By: njormrod@fb.com

Subscribers: mathieubaudet, philipp, meyering, njormrod

FB internal diff: D1575574

Blame Revision: D1575190

diff --git a/folly/small_vector.h b/folly/small_vector.h
index 92a5a2fc29d3b1561476273f2377e8e51fcf2370..132ff241ee51621e8b3900ee7e7c4e9d449bff29 100644 (file)
--- a/folly/small_vector.h
+++ b/folly/small_vector.h
@@ -391,13 +391,16 @@ public:
     try {
       std::uninitialized_copy(o.begin(), o.end(), begin());
     } catch (...) {
-      if (this->isExtern()) u.freeHeap();
+      if (this->isExtern()) {
+        u.freeHeap();
+      }
       throw;
     }
     this->setSize(n);
   }
 
-  small_vector(small_vector&& o) {
+  small_vector(small_vector&& o)
+  noexcept(std::is_nothrow_move_constructible<Value>::value) {
     if (o.isExtern()) {
       swap(o);
     } else {
@@ -877,18 +880,31 @@ private:
     auto distance = std::distance(first, last);
     makeSize(distance);
     this->setSize(distance);
-
-    detail::populateMemForward(data(), distance,
-      [&] (void* p) { new (p) value_type(*first++); }
-    );
+    try {
+      detail::populateMemForward(data(), distance,
+        [&] (void* p) { new (p) value_type(*first++); }
+      );
+    } catch (...) {
+      if (this->isExtern()) {
+        u.freeHeap();
+      }
+      throw;
+    }
   }
 
   void doConstruct(size_type n, value_type const& val) {
     makeSize(n);
     this->setSize(n);
-    detail::populateMemForward(data(), n,
-      [&] (void* p) { new (p) value_type(val); }
-    );
+    try {
+      detail::populateMemForward(data(), n,
+        [&] (void* p) { new (p) value_type(val); }
+      );
+    } catch (...) {
+      if (this->isExtern()) {
+        u.freeHeap();
+      }
+      throw;
+    }
   }
 
   // The true_type means we should forward to the size_t,value_type

diff --git a/folly/test/PackedSyncPtrTest.cpp b/folly/test/PackedSyncPtrTest.cpp
index adcc3504f71256f721f2835caaecb335b0f486c9..f926d6ff287c1bba11ade4c2b9201271628be923 100644 (file)
--- a/folly/test/PackedSyncPtrTest.cpp
+++ b/folly/test/PackedSyncPtrTest.cpp
@@ -61,6 +61,7 @@ TEST(PackedSyncPtr, Basic) {
   EXPECT_EQ(sp.extra(), 0x13);
   EXPECT_EQ(sp.get(), newP);
   sp.unlock();
+  delete sp.get();
 }
 
 // Here we use the PackedSyncPtr to lock the whole SyncVec (base, *base, and sz)
@@ -68,6 +69,7 @@ template<typename T>
 struct SyncVec {
   PackedSyncPtr<T> base;
   SyncVec() { base.init(); }
+  ~SyncVec() { free(base.get()); }
   void push_back(const T& t) {
     base.set((T*) realloc(base.get(),
       (base.extra() + 1) * sizeof(T)));

diff --git a/folly/test/ThreadLocalTest.cpp b/folly/test/ThreadLocalTest.cpp
index 36917139a23880704572367e063d3f25dd2df2d6..e2911744e47937357086c76d52bacc35c8dae847 100644 (file)
--- a/folly/test/ThreadLocalTest.cpp
+++ b/folly/test/ThreadLocalTest.cpp
@@ -184,7 +184,7 @@ TEST(ThreadLocal, SimpleRepeatDestructor) {
 
 TEST(ThreadLocal, InterleavedDestructors) {
   Widget::totalVal_ = 0;
-  ThreadLocal<Widget>* w = nullptr;
+  std::unique_ptr<ThreadLocal<Widget>> w;
   int wVersion = 0;
   const int wVersionMax = 2;
   int thIter = 0;
@@ -214,8 +214,7 @@ TEST(ThreadLocal, InterleavedDestructors) {
     {
       std::lock_guard<std::mutex> g(lock);
       thIterPrev = thIter;
-      delete w;
-      w = new ThreadLocal<Widget>();
+      w.reset(new ThreadLocal<Widget>());
       ++wVersion;
     }
     while (true) {