From ad4fc77306b08a76cf3ec19e249e811b566dd464 Mon Sep 17 00:00:00 2001
From: Jason Chan <jason.chan@mongodb.com>
Date: Tue, 25 Feb 2020 19:37:37 +0000
Subject: SERVER-46238 Transaction will abort properly when expiring in the
 middle of commitTransaction

---
 jstests/replsets/txn_expires_during_commit.js | 42 +++++++++++++++++++++++++++
 src/mongo/db/commands/txn_cmds.cpp            | 13 +++++++++
 src/mongo/db/session.cpp                      | 10 +++----
 3 files changed, 60 insertions(+), 5 deletions(-)
 create mode 100644 jstests/replsets/txn_expires_during_commit.js

diff --git a/jstests/replsets/txn_expires_during_commit.js b/jstests/replsets/txn_expires_during_commit.js
new file mode 100644
index 00000000000..43a396466cf
--- /dev/null
+++ b/jstests/replsets/txn_expires_during_commit.js
@@ -0,0 +1,42 @@
+/**
+ * Tests that a transaction will be aborted properly if it expires in the middle of a
+ * commitTransaction.
+ *
+ * @tags: [uses_transactions]
+ */
+
+(function() {
+    "use strict";
+
+    const replTest = new ReplSetTest({nodes: 1});
+    replTest.startSet();
+    replTest.initiate();
+
+    const primary = replTest.getPrimary();
+    assert.commandWorked(
+        primary.adminCommand({setParameter: 1, transactionLifetimeLimitSeconds: 5}));
+
+    const session = primary.getDB("test").getMongo().startSession();
+    const sessionDb = session.getDatabase("test");
+    assert.commandWorked(sessionDb.createCollection("c"));
+
+    // Hang in the middle of the commitTransaction command, before the transaction is committed on
+    // the session.
+    assert.commandWorked(sessionDb.adminCommand({
+        configureFailPoint: "sleepBeforeCommitTransaction",
+        mode: "alwaysOn",
+        data: {sleepMillis: NumberInt(999999)}
+    }));
+    session.startTransaction();
+    assert.commandWorked(sessionDb.c.insert({a: 1}));
+    // We expect the commitTransction operation to be killed and that the transaction is aborted
+    // cleanly.
+    assert.commandFailedWithCode(
+        sessionDb.adminCommand({commitTransaction: 1, writeConcern: {w: "majority"}}),
+        ErrorCodes.ExceededTimeLimit);
+
+    // Update the transaction state in the shell.
+    assert.commandFailedWithCode(session.commitTransaction_forTesting(),
+                                 ErrorCodes.NoSuchTransaction);
+    replTest.stopSet();
+})();
diff --git a/src/mongo/db/commands/txn_cmds.cpp b/src/mongo/db/commands/txn_cmds.cpp
index 62fabeb6277..e9a8541a2f6 100644
--- a/src/mongo/db/commands/txn_cmds.cpp
+++ b/src/mongo/db/commands/txn_cmds.cpp
@@ -42,6 +42,10 @@
 #include "mongo/db/service_context.h"
 #include "mongo/db/session_catalog.h"
 #include "mongo/util/fail_point_service.h"
+#include "mongo/util/log.h"
+
+// Do a sleep right before calling commitTransaction on the session.
+MONGO_FAIL_POINT_DEFINE(sleepBeforeCommitTransaction);
 
 namespace mongo {
 namespace {
@@ -80,6 +84,15 @@ public:
         uassert(
             ErrorCodes::CommandFailed, "commitTransaction must be run within a session", session);
 
+        MONGO_FAIL_POINT_BLOCK(sleepBeforeCommitTransaction, options) {
+            const BSONObj& data = options.getData();
+            const auto sleepMillis = data["sleepMillis"].Int();
+            log() << "sleepBeforeCommitTransaction failpoint enabled - sleeping for " << sleepMillis
+                  << " milliseconds.";
+            // Make sure we are interruptible.
+            opCtx->sleepFor(Milliseconds(sleepMillis));
+        }
+
         // commitTransaction is retryable.
         if (session->transactionIsCommitted()) {
             // We set the client last op to the last optime observed by the system to ensure that
diff --git a/src/mongo/db/session.cpp b/src/mongo/db/session.cpp
index f58071ac843..c94ea1bfba0 100644
--- a/src/mongo/db/session.cpp
+++ b/src/mongo/db/session.cpp
@@ -901,11 +901,6 @@ void Session::abortActiveTransaction(OperationContext* opCtx) {
     stdx::lock_guard<stdx::mutex> lock(_mutex);
 
     invariant(!_txnResourceStash);
-    if (_txnState != MultiDocumentTransactionState::kInProgress) {
-        return;
-    }
-
-    _abortTransaction(lock);
     {
         stdx::lock_guard<Client> clientLock(*opCtx->getClient());
         // Abort the WUOW. We should be able to abort empty transactions that don't have WUOW.
@@ -918,6 +913,11 @@ void Session::abortActiveTransaction(OperationContext* opCtx) {
                                WriteUnitOfWork::RecoveryUnitState::kNotInUnitOfWork);
         opCtx->lockState()->unsetMaxLockTimeout();
     }
+    if (_txnState != MultiDocumentTransactionState::kInProgress) {
+        return;
+    }
+
+    _abortTransaction(lock);
     {
         stdx::lock_guard<stdx::mutex> ls(_statsMutex);
         // Add the latest operation stats to the aggregate OpDebug object stored in the
-- 
cgit v1.2.1