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

Summary: I tried running folly/test with clang:dev and asan and found these leaks.

Test Plan:
unit tests with clang:dev/asan

Reviewed By: davejwatson@fb.com

Subscribers: davejwatson, trunkagent, mathieubaudet, njormrod, meyering, philipp

FB internal diff: D1569246

diff --git a/folly/SocketAddress.cpp b/folly/SocketAddress.cpp
index 4834a134a86307f3411eb4ca0fae3a37f5457dc7..6c83e033fe3b8178535aa70bd24e0255404085e8 100644 (file)
--- a/folly/SocketAddress.cpp
+++ b/folly/SocketAddress.cpp
@@ -201,12 +201,11 @@ void SocketAddress::setFromLocalAddress(int socket) {
 }
 
 void SocketAddress::setFromSockaddr(const struct sockaddr* address) {
+  uint16_t port;
   if (address->sa_family == AF_INET) {
-    storage_.addr = folly::IPAddress(address);
-    port_ = ntohs(((sockaddr_in*)address)->sin_port);
+    port = ntohs(((sockaddr_in*)address)->sin_port);
   } else if (address->sa_family == AF_INET6) {
-    storage_.addr = folly::IPAddress(address);
-    port_ = ntohs(((sockaddr_in6*)address)->sin6_port);
+    port = ntohs(((sockaddr_in6*)address)->sin6_port);
   } else if (address->sa_family == AF_UNIX) {
     // We need an explicitly specified length for AF_UNIX addresses,
     // to be able to distinguish anonymous addresses from addresses
@@ -220,7 +219,12 @@ void SocketAddress::setFromSockaddr(const struct sockaddr* address) {
       "SocketAddress::setFromSockaddr() called "
       "with unsupported address type");
   }
-  external_ = false;
+  if (getFamily() == AF_UNIX) {
+    storage_.un.free();
+    external_ = false;
+  }
+  storage_.addr = folly::IPAddress(address);
+  port_ = port;
 }
 
 void SocketAddress::setFromSockaddr(const struct sockaddr* address,

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