(folly) fix memory leak in folly::Observer addCallback

Summary: shared_ptr cycle triggered by inconsistent destructor/move constructor

Reviewed By: yfeldblum, andriigrynenko

Differential Revision: D19206912

fbshipit-source-id: 2b5dcf639eb2e8f9d4ade9511cd9093d7565d482

folly/experimental/observer/Observer-inl.h

@@ -98,6 +98,13 @@ CallbackHandle::CallbackHandle(
   });
 }
 
+inline CallbackHandle& CallbackHandle::operator=(
+    CallbackHandle&& handle) noexcept {
+  cancel();
+  context_ = std::move(handle.context_);
+  return *this;
+}
+
 inline CallbackHandle::~CallbackHandle() {
   cancel();
 }

folly/experimental/observer/Observer.h

@@ -129,7 +129,7 @@ class CallbackHandle {
   CallbackHandle(const CallbackHandle&) = delete;
   CallbackHandle(CallbackHandle&&) = default;
   CallbackHandle& operator=(const CallbackHandle&) = delete;
-  CallbackHandle& operator=(CallbackHandle&&) = default;
+  CallbackHandle& operator=(CallbackHandle&&) noexcept;
   ~CallbackHandle();
 
   // If callback is currently running, waits until it completes.

folly/experimental/observer/test/ObserverTest.cpp

@@ -375,6 +375,14 @@ TEST(Observer, SetCallback) {
   EXPECT_EQ(2, callbackCallsCount);
 }
 
+TEST(Observer, CallbackMemoryLeak) {
+  folly::observer::SimpleObservable<int> observable(42);
+  auto observer = observable.getObserver();
+  auto callbackHandle = observer.addCallback([](auto) {});
+  // should not leak
+  callbackHandle = observer.addCallback([](auto) {});
+}
+
 int makeObserverRecursion(int n) {
   if (n == 0) {
     return 0;