fix(auth-utils): move destroyed check inside mutex for atomic validation
CRITICAL FIX: Addresses Copilot AI comment on PR #79 about race condition between destroyed flag check and mutex acquisition. Problem: The destroyed flag check happened OUTSIDE the mutex, creating a race window between check and mutex acquisition. destroy() could complete after check passes but before mutex is acquired, leading to transaction using destroyed resources. Timeline before fix: T0: transaction() line 307 - check destroyed (false) ✅ T1: destroy() line 350 - destroyed = true T2: destroy() line 379 - preKeyManager.destroy() T3: transaction() line 324 - mutex.runExclusive() acquires mutex T4: work() executes → uses destroyed resources → CRASH Changes: - Removed destroyed check from line 306-309 (outside mutex) - Added destroyed check inside mutex.runExclusive() at line 320-324 - Check now happens atomically within mutex protection - If mutex acquired, resources guaranteed to exist Timeline after fix: T0: transaction() line 315 - getTxMutex(key) T1: destroy() line 350 - destroyed = true T2: destroy() line 358 - checks mutex (locked by transaction) T3: destroy() line 370 - early return (resources NOT destroyed) T4: transaction() line 319 - mutex.runExclusive() executes T5: transaction() line 322 - check destroyed → true → throws error ✅ OR (if check happens first): T0: transaction() line 319 - mutex.runExclusive() acquires mutex T1: transaction() line 322 - check destroyed (false) ✅ T2: destroy() called → line 358 checks mutex (LOCKED) T3: destroy() early returns (resources safe) T4: transaction() completes successfully Validation: - ✅ Check is now atomic (inside mutex critical section) - ✅ No window between check and resource usage - ✅ destroy() respects locked mutex (existing protection) - ✅ Either transaction completes OR gets rejected, never crashes https://claude.ai/code/session_VMxqX
This commit is contained in:
@@ -303,11 +303,6 @@ export const addTransactionCapability = (
|
||||
isInTransaction,
|
||||
|
||||
transaction: async (work, key) => {
|
||||
// CRITICAL: Prevent transactions after destroy to avoid use-after-free
|
||||
if (destroyed) {
|
||||
throw new Error('Transaction capability destroyed - socket closed')
|
||||
}
|
||||
|
||||
const existing = txStorage.getStore()
|
||||
|
||||
// Nested transaction - reuse existing context
|
||||
@@ -322,6 +317,12 @@ export const addTransactionCapability = (
|
||||
|
||||
try {
|
||||
return await mutex.runExclusive(async () => {
|
||||
// CRITICAL: Check destroyed flag INSIDE mutex to prevent race condition
|
||||
// This ensures atomic check-and-execute: if we acquire mutex, resources exist
|
||||
if (destroyed) {
|
||||
throw new Error('Transaction capability destroyed - socket closed')
|
||||
}
|
||||
|
||||
const ctx: TransactionContext = {
|
||||
cache: {},
|
||||
mutations: {},
|
||||
|
||||
Reference in New Issue
Block a user