Skip to content

Commit a7cf30d

Browse files
authored
fix(db-mongodb,drizzle): prevent race condition in transaction session cleanup (#14651)
## Description Adds defensive checks to prevent `MongoExpiredSessionError` when transaction sessions are accessed incorrectly or at bad timing. ## Why Adds defensive layers against transaction session misuse. We already check if sessions exist in `db.sessions` before use, but this PR enhances safety by: 1. **Reducing race condition windows** - Delete sessions from registry before ending them, not after 2. **Validating session state** - Check `inTransaction()` to catch sessions that were ended but not deleted 3. **Isolating cached objects** - Shallow copy prevents shared mutation of cached `req` objects, which helps to prevent improper `req.transactionID `sharing in the first place These changes don't prevent root causes (leaked transactionIDs, improper req sharing during parallel operations) but provide graceful degradation. Operations that would crash with `MongoExpiredSessionError` when accessing stale sessions now fall back to running without a session. ## Changes ### 1. Delete sessions before ending them ```ts // Before: Other operations could retrieve session during endSession() await session.endSession() delete this.sessions[id] ``` ```ts // After: Remove from registry first delete this.sessions[id] await session.endSession() ``` ### 2. Validate sessions before use Check `session.inTransaction()` in `getSession` - if ended, return `undefined` instead of expired session. This helps prevent using a session that has already ended in the potential window where a session has expired, but has not yet been removed from the `sessions` array. ### 3. Shallow copy cached req Prevent mutations to cached `req` objects returned by `initReq` from affecting parallel operations.
1 parent 294ebf5 commit a7cf30d

File tree

7 files changed

+116
-66
lines changed

7 files changed

+116
-66
lines changed

packages/db-mongodb/src/transactions/commitTransaction.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,30 @@ import type { MongooseAdapter } from '../index.js'
44

55
export const commitTransaction: CommitTransaction = async function commitTransaction(
66
this: MongooseAdapter,
7-
id,
7+
incomingID = '',
88
) {
9-
if (id instanceof Promise) {
9+
const transactionID = incomingID instanceof Promise ? await incomingID : incomingID
10+
11+
if (!this.sessions[transactionID]) {
1012
return
1113
}
1214

13-
if (!this.sessions[id]?.inTransaction()) {
15+
if (!this.sessions[transactionID]?.inTransaction()) {
16+
// Clean up the orphaned session reference
17+
delete this.sessions[transactionID]
1418
return
1519
}
1620

17-
await this.sessions[id].commitTransaction()
21+
const session = this.sessions[transactionID]
22+
23+
// Delete from registry FIRST to prevent race conditions
24+
// This ensures other operations can't retrieve this session while we're ending it
25+
delete this.sessions[transactionID]
26+
27+
await session.commitTransaction()
1828
try {
19-
await this.sessions[id].endSession()
29+
await session.endSession()
2030
} catch (_) {
2131
// ending sessions is only best effort and won't impact anything if it fails since the transaction was committed
2232
}
23-
delete this.sessions[id]
2433
}

packages/db-mongodb/src/transactions/rollbackTransaction.ts

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,7 @@ export const rollbackTransaction: RollbackTransaction = async function rollbackT
66
this: MongooseAdapter,
77
incomingID = '',
88
) {
9-
let transactionID: number | string
10-
11-
if (incomingID instanceof Promise) {
12-
transactionID = await incomingID
13-
} else {
14-
transactionID = incomingID
15-
}
9+
const transactionID = incomingID instanceof Promise ? await incomingID : incomingID
1610

1711
// if multiple operations are using the same transaction, the first will flow through and delete the session.
1812
// subsequent calls should be ignored.
@@ -27,12 +21,17 @@ export const rollbackTransaction: RollbackTransaction = async function rollbackT
2721
return
2822
}
2923

24+
const session = this.sessions[transactionID]
25+
26+
// Delete from registry FIRST to prevent race conditions
27+
// This ensures other operations can't retrieve this session while we're aborting it
28+
delete this.sessions[transactionID]
29+
3030
// the first call for rollback should be aborted and deleted causing any other operations with the same transaction to fail
3131
try {
32-
await this.sessions[transactionID]?.abortTransaction()
33-
await this.sessions[transactionID]?.endSession()
34-
} catch (error) {
32+
await session.abortTransaction()
33+
await session.endSession()
34+
} catch (_error) {
3535
// ignore the error as it is likely a race condition from multiple errors
3636
}
37-
delete this.sessions[transactionID]
3837
}

packages/db-mongodb/src/utilities/getSession.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,21 @@ export async function getSession(
2222
}
2323

2424
if (transactionID) {
25-
return db.sessions[transactionID]
25+
const session = db.sessions[transactionID]
26+
27+
// Defensive check for race conditions where:
28+
// 1. Session was retrieved from db.sessions
29+
// 2. Another operation committed/rolled back and ended the session
30+
// 3. This operation tries to use the now-ended session
31+
// Note: This shouldn't normally happen as sessions are deleted from db.sessions
32+
// after commit/rollback, but can occur due to async timing where we hold
33+
// a reference to a session object that gets ended before we use it.
34+
if (session && !session.inTransaction()) {
35+
// Clean up the orphaned session reference
36+
delete db.sessions[transactionID]
37+
return undefined
38+
}
39+
40+
return session
2641
}
2742
}
Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,24 @@
11
import type { CommitTransaction } from 'payload'
22

3-
export const commitTransaction: CommitTransaction = async function commitTransaction(id) {
4-
if (id instanceof Promise) {
5-
return
6-
}
3+
export const commitTransaction: CommitTransaction = async function commitTransaction(
4+
incomingID = '',
5+
) {
6+
const transactionID = incomingID instanceof Promise ? await incomingID : incomingID
77

88
// if the session was deleted it has already been aborted
9-
if (!this.sessions[id]) {
9+
if (!this.sessions[transactionID]) {
1010
return
1111
}
1212

13+
const session = this.sessions[transactionID]
14+
15+
// Delete from registry FIRST to prevent race conditions
16+
// This ensures other operations can't retrieve this session while we're ending it
17+
delete this.sessions[transactionID]
18+
1319
try {
14-
await this.sessions[id].resolve()
20+
await session.resolve()
1521
} catch (_) {
16-
await this.sessions[id].reject()
22+
await session.reject()
1723
}
18-
19-
delete this.sessions[id]
2024
}

packages/drizzle/src/transactions/rollbackTransaction.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,12 @@ export const rollbackTransaction: RollbackTransaction = async function rollbackT
1111
return
1212
}
1313

14-
// end the session promise in failure by calling reject
15-
await this.sessions[transactionID].reject()
14+
const session = this.sessions[transactionID]
1615

17-
// delete the session causing any other operations with the same transaction to fail
16+
// Delete from registry FIRST to prevent race conditions
17+
// This ensures other operations can't retrieve this session while we're ending it
1818
delete this.sessions[transactionID]
19+
20+
// end the session promise in failure by calling reject
21+
await session.reject()
1922
}

packages/drizzle/src/utilities/getTransaction.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@ import type { DrizzleAdapter } from '../types.js'
44

55
/**
66
* Returns current db transaction instance from req or adapter.drizzle itself
7+
*
8+
* If a transaction session doesn't exist (e.g., it was already committed/rolled back),
9+
* falls back to the default adapter.drizzle instance to prevent errors.
710
*/
811
export const getTransaction = async <T extends DrizzleAdapter = DrizzleAdapter>(
912
adapter: T,

packages/next/src/utilities/initReq.ts

Lines changed: 53 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -94,43 +94,60 @@ export const initReq = async function ({
9494
}
9595
}, 'global')
9696

97-
return reqCache.get(async () => {
98-
const { i18n, languageCode, payload, responseHeaders, user } = partialResult
99-
100-
const { req: reqOverrides, ...optionsOverrides } = overrides || {}
101-
102-
const req = await createLocalReq(
103-
{
97+
return reqCache
98+
.get(async () => {
99+
const { i18n, languageCode, payload, responseHeaders, user } = partialResult
100+
101+
const { req: reqOverrides, ...optionsOverrides } = overrides || {}
102+
103+
const req = await createLocalReq(
104+
{
105+
req: {
106+
headers,
107+
host: headers.get('host'),
108+
i18n: i18n as I18n,
109+
responseHeaders,
110+
user,
111+
...(reqOverrides || {}),
112+
},
113+
...(optionsOverrides || {}),
114+
},
115+
payload,
116+
)
117+
118+
const locale = await getRequestLocale({
119+
req,
120+
})
121+
122+
req.locale = locale?.code
123+
124+
const permissions = await getAccessResults({
125+
req,
126+
})
127+
128+
return {
129+
cookies,
130+
headers,
131+
languageCode,
132+
locale,
133+
permissions,
134+
req,
135+
}
136+
}, key)
137+
.then((result) => {
138+
// CRITICAL: Create a shallow copy of req before returning to prevent
139+
// mutations from propagating to the cached req object.
140+
// This ensures parallel operations using the same cache key don't affect each other.
141+
return {
142+
...result,
104143
req: {
105-
headers,
106-
host: headers.get('host'),
107-
i18n: i18n as I18n,
108-
responseHeaders,
109-
user,
110-
...(reqOverrides || {}),
144+
...result.req,
145+
...(result.req?.context
146+
? {
147+
context: { ...result.req.context },
148+
}
149+
: {}),
111150
},
112-
...(optionsOverrides || {}),
113-
},
114-
payload,
115-
)
116-
117-
const locale = await getRequestLocale({
118-
req,
119-
})
120-
121-
req.locale = locale?.code
122-
123-
const permissions = await getAccessResults({
124-
req,
151+
}
125152
})
126-
127-
return {
128-
cookies,
129-
headers,
130-
languageCode,
131-
locale,
132-
permissions,
133-
req,
134-
}
135-
}, key)
136153
}

0 commit comments

Comments
 (0)