SERVER-29098 ShardingTaskExecutor::scheduleRemoteCommand should not run passed callback before processing metadata

2 files changed, 11 insertions, 1 deletions

diff --git a/src/mongo/db/s/sharding_task_executor.cpp b/src/mongo/db/s/sharding_task_executor.cpp
index 17f5b5fd861..75cdd4b7a32 100644
--- a/src/mongo/db/s/sharding_task_executor.cpp
+++ b/src/mongo/db/s/sharding_task_executor.cpp
@@ -41,6 +41,7 @@
 #include "mongo/rpc/metadata/sharding_metadata.h"
 #include "mongo/s/cluster_last_error_info.h"
 #include "mongo/util/log.h"
+#include "mongo/util/scopeguard.h"
 
 namespace mongo {
 namespace executor {
@@ -117,7 +118,7 @@ StatusWith<TaskExecutor::CallbackHandle> ShardingTaskExecutor::scheduleRemoteCom
 
     auto shardingCb = [timeTracker, clusterGLE, request, cb](
         const TaskExecutor::RemoteCommandCallbackArgs& args) {
-        cb(args);
+        ON_BLOCK_EXIT([&cb, &args]() { cb(args); });
 
         if (!args.response.isOK()) {
             LOG(1) << "Error processing the remote request, not updating operationTime or gLE";
diff --git a/src/mongo/executor/task_executor.h b/src/mongo/executor/task_executor.h
index 04f911da5dc..0c7a676a358 100644
--- a/src/mongo/executor/task_executor.h
+++ b/src/mongo/executor/task_executor.h
@@ -190,6 +190,9 @@ public:
      * ErrorCodes::ShutdownInProgress.
      *
      * May be called by client threads or callbacks running in the executor.
+     *
+     * Contract: Implementations should guarantee that callback should be called *after* doing any
+     * processing related to the callback.
      */
     virtual StatusWith<CallbackHandle> scheduleWork(const CallbackFn& work) = 0;
 
@@ -200,6 +203,9 @@ public:
      * ErrorCodes::ShutdownInProgress.
      *
      * May be called by client threads or callbacks running in the executor.
+     *
+     * Contract: Implementations should guarantee that callback should be called *after* doing any
+     * processing related to the callback.
      */
     virtual StatusWith<CallbackHandle> scheduleWorkAt(Date_t when, const CallbackFn& work) = 0;
 
@@ -211,6 +217,9 @@ public:
      * ErrorCodes::ShutdownInProgress.
      *
      * May be called by client threads or callbacks running in the executor.
+     *
+     * Contract: Implementations should guarantee that callback should be called *after* doing any
+     * processing related to the callback.
      */
     virtual StatusWith<CallbackHandle> scheduleRemoteCommand(const RemoteCommandRequest& request,
                                                              const RemoteCommandCallbackFn& cb) = 0;