Reset context shared_ptr in AsyncTimeout::cancelTimeout()

Summary:
This seems to fix issue in the attached task.

`context_` is set in `scheduleTimeout()` and never gets reset. So when an AsyncTimeout object is held across requests by objects like `IOThreadPool0` in I/O Thread Pool used by wangle/acceptor/ConnectionManager, RequestContext object created for the last request is leaked until `IOThreadPool0` handles another request and overrides `context_`. In the issue described in attached task, unit test has single request, next request never comes in, RequestContext doesn't end until test service stops, logging feature relying on RequestContext dtor doesn't get called in time, and thus unit test fails.

Reviewed By: yfeldblum

Differential Revision: D6402268

fbshipit-source-id: 200c6d358dfa6d7d9aa68ab05f6f1c7f4117b0ec

diff --git a/folly/io/async/AsyncTimeout.cpp b/folly/io/async/AsyncTimeout.cpp
index 680d64c..ad5cae4 100644 (file)
--- a/folly/io/async/AsyncTimeout.cpp
+++ b/folly/io/async/AsyncTimeout.cpp
@@ -90,6 +90,7 @@ bool AsyncTimeout::scheduleTimeout(uint32_t milliseconds) {
 void AsyncTimeout::cancelTimeout() {
   if (isScheduled()) {
     timeoutManager_->cancelTimeout(this);
+    context_.reset();
   }
 }
 

diff --git a/folly/io/async/test/AsyncTimeoutTest.cpp b/folly/io/async/test/AsyncTimeoutTest.cpp
index 1d7c04f..3b16709 100644 (file)
--- a/folly/io/async/test/AsyncTimeoutTest.cpp
+++ b/folly/io/async/test/AsyncTimeoutTest.cpp
@@ -77,8 +77,18 @@ TEST(AsyncTimeout, cancel_make) {
     [&]() noexcept { value = expected; }
   );
 
-  observer->scheduleTimeout(std::chrono::milliseconds(100));
-  observer->cancelTimeout();
+  std::weak_ptr<RequestContext> rctx_weak_ptr;
+
+  {
+    RequestContextScopeGuard rctx_guard;
+    rctx_weak_ptr = RequestContext::saveContext();
+    observer->scheduleTimeout(std::chrono::milliseconds(100));
+    observer->cancelTimeout();
+  }
+
+  // Ensure that RequestContext created for the scope has been released and
+  // deleted.
+  EXPECT_EQ(rctx_weak_ptr.expired(), true);
 
   manager.loop();
 
@@ -89,14 +99,24 @@ TEST(AsyncTimeout, cancel_schedule) {
   int value = 0;
   int const expected = 10;
   EventBase manager;
+  std::unique_ptr<AsyncTimeout> observer;
+  std::weak_ptr<RequestContext> rctx_weak_ptr;
 
-  auto observer = AsyncTimeout::schedule(
-    std::chrono::milliseconds(100),
-    manager,
-    [&]() noexcept { value = expected; }
-  );
+  {
+    RequestContextScopeGuard rctx_guard;
+    rctx_weak_ptr = RequestContext::saveContext();
+
+    observer = AsyncTimeout::schedule(
+        std::chrono::milliseconds(100), manager, [&]() noexcept {
+          value = expected;
+        });
+
+    observer->cancelTimeout();
+  }
 
-  observer->cancelTimeout();
+  // Ensure that RequestContext created for the scope has been released and
+  // deleted.
+  EXPECT_EQ(rctx_weak_ptr.expired(), true);
 
   manager.loop();