[SERVER-83453] Release TPTE iterators while holding the mutex Created: 20/Nov/23  Updated: 29/Nov/23  Resolved: 29/Nov/23

Status: Closed
Project: Core Server
Component/s: Internal Code
Affects Version/s: None
Fix Version/s: 7.3.0-rc0

Type: Task Priority: Major - P3
Reporter: Amirsaman Memaripour Assignee: Amirsaman Memaripour
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Depends
Backwards Compatibility: Fully Compatible
Sprint: Service Arch 2023-11-27, Service Arch 2023-12-11
Participants:
Linked BF Score: 130

 Description   

This is more of a temporary fix to evaluate a hypothesis: destroying iterators without synchronizing with the corresponding list container can result into a data-race in DEBUG builds. This may show up as a read-after-free in ASAN builds, and a segmentation fault otherwise.

The following wraps the iterators in optional to ensure they are always destroyed before releasing the ThreadPoolTaskExecutor mutex – I couldn't find any other iterators that were created/destroyed without holding this mutex, and applying the following patch seems to fix the issue.

diff --git a/src/mongo/executor/thread_pool_task_executor.cpp b/src/mongo/executor/thread_pool_task_executor.cpp
index 5be6b56a2cb..5fdf50f7c8b 100644
--- a/src/mongo/executor/thread_pool_task_executor.cpp
+++ b/src/mongo/executor/thread_pool_task_executor.cpp
@@ -28,6 +28,7 @@
  */
 
 
+#include "boost/optional/optional.hpp"
 #include "mongo/platform/basic.h"
 
 #include "mongo/executor/thread_pool_task_executor.h"
@@ -584,18 +585,21 @@ void ThreadPoolTaskExecutor::scheduleIntoPool_inlock(WorkQueue* fromQueue,
 }
 
 void ThreadPoolTaskExecutor::scheduleIntoPool_inlock(WorkQueue* fromQueue,
-                                                     const WorkQueue::iterator& iter,
+                                                     boost::optional<WorkQueue::iterator> iter,
                                                      stdx::unique_lock<Latch> lk) {
-    scheduleIntoPool_inlock(fromQueue, iter, std::next(iter), std::move(lk));
+    boost::optional<WorkQueue::iterator> end{std::next(*iter)};
+    scheduleIntoPool_inlock(fromQueue, std::move(iter), std::move(end), std::move(lk));
 }
 
 void ThreadPoolTaskExecutor::scheduleIntoPool_inlock(WorkQueue* fromQueue,
-                                                     const WorkQueue::iterator& begin,
-                                                     const WorkQueue::iterator& end,
+                                                     boost::optional<WorkQueue::iterator> begin,
+                                                     boost::optional<WorkQueue::iterator> end,
                                                      stdx::unique_lock<Latch> lk) {
     dassert(fromQueue != &_poolInProgressQueue);
-    std::vector<std::shared_ptr<CallbackState>> todo(begin, end);
-    _poolInProgressQueue.splice(_poolInProgressQueue.end(), *fromQueue, begin, end);
+    std::vector<std::shared_ptr<CallbackState>> todo(*begin, *end);
+    _poolInProgressQueue.splice(_poolInProgressQueue.end(), *fromQueue, *begin, *end);
+    begin = boost::none;
+    end = boost::none;
 
     lk.unlock();
 
diff --git a/src/mongo/executor/thread_pool_task_executor.h b/src/mongo/executor/thread_pool_task_executor.h
index 9c95467118f..b0766d75d52 100644
--- a/src/mongo/executor/thread_pool_task_executor.h
+++ b/src/mongo/executor/thread_pool_task_executor.h
@@ -172,7 +172,7 @@ private:
      * _poolInProgressQueue.
      */
     void scheduleIntoPool_inlock(WorkQueue* fromQueue,
-                                 const WorkQueue::iterator& iter,
+                                 boost::optional<WorkQueue::iterator> iter,
                                  stdx::unique_lock<Latch> lk);
 
     /**
@@ -180,8 +180,8 @@ private:
      * and moves them into _poolInProgressQueue.
      */
     void scheduleIntoPool_inlock(WorkQueue* fromQueue,
-                                 const WorkQueue::iterator& begin,
-                                 const WorkQueue::iterator& end,
+                                 boost::optional<WorkQueue::iterator> begin,
+                                 boost::optional<WorkQueue::iterator> end,
                                  stdx::unique_lock<Latch> lk);
 
     /**



 Comments   
Comment by Githook User [ 29/Nov/23 ]

Author:

{'name': 'Amirsaman Memaripour', 'email': 'amirsaman.memaripour@mongodb.com', 'username': 'samanca'}

Message: SERVER-83453 Release TPTE iterators while holding the mutex
Branch: master
https://github.com/mongodb/mongo/commit/b4f41256a18734fb025cb3ea739eb1f04b776886

Generated at Thu Feb 08 06:52:14 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.