Skip to content

Commit

Permalink
correct timing issues around transactions
Browse files Browse the repository at this point in the history
`endSession` must wait for all the machinery behind the scenes to
check out a connection and write a message before considering its
job finished.
  • Loading branch information
mbroadst committed Sep 9, 2020
1 parent 7b1d9bd commit e340b8f
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 40 deletions.
64 changes: 29 additions & 35 deletions lib/core/sessions.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const Transaction = require('./transactions').Transaction;
const TxnState = require('./transactions').TxnState;
const isPromiseLike = require('./utils').isPromiseLike;
const ReadPreference = require('./topologies/read_preference');
const maybePromise = require('../utils').maybePromise;
const isTransactionCommand = require('./transactions').isTransactionCommand;
const resolveClusterTime = require('./topologies/shared').resolveClusterTime;
const isSharded = require('./wireprotocol/shared').isSharded;
Expand Down Expand Up @@ -125,25 +126,36 @@ class ClientSession extends EventEmitter {
if (typeof options === 'function') (callback = options), (options = {});
options = options || {};

if (this.hasEnded) {
if (typeof callback === 'function') callback(null, null);
return;
}
const session = this;
return maybePromise(this, callback, done => {
if (session.hasEnded) {
return done();
}

if (this.serverSession && this.inTransaction()) {
this.abortTransaction(); // pass in callback?
}
function completeEndSession() {
// release the server session back to the pool
session.sessionPool.release(session.serverSession);
session[kServerSession] = undefined;

// release the server session back to the pool
this.sessionPool.release(this.serverSession);
this[kServerSession] = undefined;
// mark the session as ended, and emit a signal
session.hasEnded = true;
session.emit('ended', session);

// spec indicates that we should ignore all errors for `endSessions`
done();
}

if (session.serverSession && session.inTransaction()) {
session.abortTransaction(err => {
if (err) return done(err);
completeEndSession();
});

// mark the session as ended, and emit a signal
this.hasEnded = true;
this.emit('ended', this);
return;
}

// spec indicates that we should ignore all errors for `endSessions`
if (typeof callback === 'function') callback(null, null);
completeEndSession();
});
}

/**
Expand Down Expand Up @@ -227,16 +239,7 @@ class ClientSession extends EventEmitter {
* @return {Promise} A promise is returned if no callback is provided
*/
commitTransaction(callback) {
if (typeof callback === 'function') {
endTransaction(this, 'commitTransaction', callback);
return;
}

return new Promise((resolve, reject) => {
endTransaction(this, 'commitTransaction', (err, reply) =>
err ? reject(err) : resolve(reply)
);
});
return maybePromise(this, callback, done => endTransaction(this, 'commitTransaction', done));
}

/**
Expand All @@ -246,16 +249,7 @@ class ClientSession extends EventEmitter {
* @return {Promise} A promise is returned if no callback is provided
*/
abortTransaction(callback) {
if (typeof callback === 'function') {
endTransaction(this, 'abortTransaction', callback);
return;
}

return new Promise((resolve, reject) => {
endTransaction(this, 'abortTransaction', (err, reply) =>
err ? reject(err) : resolve(reply)
);
});
return maybePromise(this, callback, done => endTransaction(this, 'abortTransaction', done));
}

/**
Expand Down
11 changes: 6 additions & 5 deletions test/functional/spec-runner/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,12 @@ function runTestSuiteTest(configuration, spec, context) {
throw err;
})
.then(() => {
if (session0) session0.endSession();
if (session1) session1.endSession();

return validateExpectations(context.commandEvents, spec, savedSessionData);
});
const promises = [];
if (session0) promises.push(session0.endSession());
if (session1) promises.push(session1.endSession());
return Promise.all(promises);
})
.then(() => validateExpectations(context.commandEvents, spec, savedSessionData));
});
}

Expand Down

0 comments on commit e340b8f

Please sign in to comment.