-
Type: Task
-
Resolution: Done
-
Priority: Major - P3
-
None
-
Affects Version/s: None
-
Component/s: Internal Code
-
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
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; })
- is depended on by
-
SERVER-52942 Evaluate whether we could change the destruction order of the closure in the ExecutorFuture class
- Closed
- is related to
-
SERVER-53108 Fix OperationContext lifetime management in ReshardingOplogApplier and use batchSize > 1
- Closed
-
SERVER-66126 Clear Future/ExecutorFuture continuations as they run
- Open
- related to
-
SERVER-54119 Destroy SharedStateBase as we unfold continuations
- Closed