Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-53538

Clarify semantics of when destructor runs for ExecutorFuture chains

    • v5.0
    • Service Arch 2021-02-22, Service Arch 2021-04-05, Service Arch 2021-04-19, Service Arch 2021-06-14, Service Arch 2021-06-28, Service Arch 2021-07-12, Service Arch 2021-08-09, Service Arch 2022-03-21, Service Arch 2022-04-04, Service Arch 2022-04-18, Service Arch 2022-05-02, Service Arch 2022-05-16
    • 176
    • 4

      While working on SERVER-53108, I had encountered an invariant failure about ServiceContext::_clients not being empty. After digging into it—and thanks to the numactl --physcpubind=0 trick—I determined that the reference count for the executor::TaskExecutor hadn't been decremented (from 2 to 1) despite there being no variables alive in the C++ code that could refer to it.

      amirsaman.memaripour and I weren't sure if this behavior was expected or not; he asked me to file this ticket.

       

      Acceptance Criteria:

      Conclude and document the semantics for the destructor and recommendations for fixing it. 

      Steps to reproduce
      numactl --physcpubind=0 build/install/bin/util_test --filter=CheckIfGetWaitsForDestructorToRun
      
      Unable to find source-code formatter for language: diff. Available languages are: actionscript, ada, applescript, bash, c, c#, c++, cpp, css, erlang, go, groovy, haskell, html, java, javascript, js, json, lua, none, nyan, objc, perl, php, python, r, rainbow, ruby, scala, sh, sql, swift, visualbasic, xml, yaml
      diff --git a/src/mongo/util/future_util_test.cpp b/src/mongo/util/future_util_test.cpp
      index e9878d21e8..d35a45ef72 100644
      --- a/src/mongo/util/future_util_test.cpp
      +++ b/src/mongo/util/future_util_test.cpp
      @@ -64,7 +64,7 @@ public:
               return _network;
           }
      
      -private:
      +protected:
           std::shared_ptr<executor::ThreadPoolTaskExecutor> _executor;
           executor::NetworkInterfaceMock* _network;
       };
      @@ -85,6 +85,26 @@ private:
      
       using AsyncTryUntilTest = FutureUtilTest;
      
      +// This test case doesn't actually use AsyncTry but AsyncTryUntilTest is a test fixture that I knew
      +// had a ThreadPoolTaskExecutor.
      +TEST_F(AsyncTryUntilTest, CheckIfGetWaitsForDestructorToRun) {
      +    auto currentUseCount = _executor.use_count();
      +    auto i = 0;
      +    {
      +        ExecutorFuture(_executor)
      +            .then([&i, executor = _executor] {
      +                (void)executor;
      +                ++i;
      +            })
      +            // Uncommenting the following line causes the test to pass.
      +            // .then([] {})
      +            .get();
      +    }
      +
      +    ASSERT_EQ(i, 1);
      +    ASSERT_EQ(_executor.use_count(), currentUseCount);
      +}
      +
       TEST_F(AsyncTryUntilTest, LoopExecutesOnceWithAlwaysTrueCondition) {
           auto i = 0;
           auto resultFut = AsyncTry([&] { ++i; })
      

            Assignee:
            matt.diener@mongodb.com Matt Diener (Inactive)
            Reporter:
            max.hirschhorn@mongodb.com Max Hirschhorn
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

              Created:
              Updated:
              Resolved: