Details
-
Task
-
Resolution: Done
-
Major - P3
-
None
-
None
-
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
Description
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; }) |
Attachments
Issue Links
- 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
-