fix(pre-key-manager): add destroyed flag with atomic protection (M2)
PROBLEM:
PreKeyManager lacked a destroyed flag, allowing operations to execute after
destroy() was called. This created race conditions where:
1. Operations could add tasks to queues after they were cleared/paused
2. New queues could be created after destroy() cleaned them up
3. Tasks could execute on destroyed resources
4. Multiple destroy() calls could cleanup resources multiple times
Race scenario 1 (Operations after destroy):
T1: processOperations() → getQueue('pre-key')
T2: destroy() → queue.clear(), queue.pause()
T1: queue.add(task) → Task added to paused queue (never executes)
Race scenario 2 (Queue recreation):
T1: destroy() → queues.clear()
T2: processOperations() → getQueue() → Creates NEW queue!
T2: queue.add(task) → Task executes (queue not destroyed)
SOLUTION:
Added destroyed flag with atomic check-and-set pattern, similar to V7/V8/M1:
1. Added private destroyed flag with comprehensive thread-safety documentation
2. Created checkDestroyed() method that throws error if destroyed
3. All public methods (processOperations, validateDeletions) check flag first
4. destroy() uses atomic check-and-set (checks flag, sets immediately)
Defense in depth:
- checkDestroyed() prevents new operations from starting
- Flag set BEFORE cleanup begins (closes race window)
- Reentrancy guard prevents multiple destroy() calls
- Clear error messages for debugging
VALIDATION:
✓ Protocolo de Análise complete (5 steps)
✓ TypeScript compilation verified (no new errors)
✓ Consistent with V7/V8/M1 atomic patterns
✓ All public entry points protected
FILES MODIFIED:
- src/Utils/pre-key-manager.ts:11-37 - Added destroyed flag and checkDestroyed()
- src/Utils/pre-key-manager.ts:60-61 - Added check in processOperations()
- src/Utils/pre-key-manager.ts:134-135 - Added check in validateDeletions()
- src/Utils/pre-key-manager.ts:160-171 - Atomic check-and-set in destroy()
https://claude.ai/code/session_01NTVq3RHgGpgKL289JGvw55
This commit is contained in:
@@ -8,11 +8,34 @@ import type { ILogger } from './logger'
|
|||||||
export class PreKeyManager {
|
export class PreKeyManager {
|
||||||
private readonly queues = new Map<string, PQueue>()
|
private readonly queues = new Map<string, PQueue>()
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Destroyed flag - protected by atomic check-and-set in destroy()
|
||||||
|
*
|
||||||
|
* THREAD SAFETY: Prevents operations from executing after destroy() is called.
|
||||||
|
* All public methods check this flag before proceeding.
|
||||||
|
*
|
||||||
|
* CRITICAL: Prevents race conditions where:
|
||||||
|
* - Operations add tasks to queues after they've been cleared/paused
|
||||||
|
* - New queues are created after destroy() has cleaned them up
|
||||||
|
* - Tasks execute on destroyed resources
|
||||||
|
*/
|
||||||
|
private destroyed = false
|
||||||
|
|
||||||
constructor(
|
constructor(
|
||||||
private readonly store: SignalKeyStore,
|
private readonly store: SignalKeyStore,
|
||||||
private readonly logger: ILogger
|
private readonly logger: ILogger
|
||||||
) {}
|
) {}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if manager has been destroyed
|
||||||
|
* @throws Error if manager has been destroyed
|
||||||
|
*/
|
||||||
|
private checkDestroyed(): void {
|
||||||
|
if (this.destroyed) {
|
||||||
|
throw new Error('PreKeyManager has been destroyed - cannot perform operations')
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get or create a queue for a specific key type
|
* Get or create a queue for a specific key type
|
||||||
*/
|
*/
|
||||||
@@ -34,6 +57,9 @@ export class PreKeyManager {
|
|||||||
mutations: SignalDataSet,
|
mutations: SignalDataSet,
|
||||||
isInTransaction: boolean
|
isInTransaction: boolean
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
// PROTECTION: Check destroyed flag before processing
|
||||||
|
this.checkDestroyed()
|
||||||
|
|
||||||
const keyData = data[keyType]
|
const keyData = data[keyType]
|
||||||
if (!keyData) return
|
if (!keyData) return
|
||||||
|
|
||||||
@@ -105,6 +131,9 @@ export class PreKeyManager {
|
|||||||
* Validate and process pre-key deletions outside transactions
|
* Validate and process pre-key deletions outside transactions
|
||||||
*/
|
*/
|
||||||
async validateDeletions(data: SignalDataSet, keyType: keyof SignalDataTypeMap): Promise<void> {
|
async validateDeletions(data: SignalDataSet, keyType: keyof SignalDataTypeMap): Promise<void> {
|
||||||
|
// PROTECTION: Check destroyed flag before processing
|
||||||
|
this.checkDestroyed()
|
||||||
|
|
||||||
const keyData = data[keyType]
|
const keyData = data[keyType]
|
||||||
if (!keyData) return
|
if (!keyData) return
|
||||||
|
|
||||||
@@ -129,6 +158,18 @@ export class PreKeyManager {
|
|||||||
* Should be called during connection cleanup to prevent memory leaks
|
* Should be called during connection cleanup to prevent memory leaks
|
||||||
*/
|
*/
|
||||||
destroy(): void {
|
destroy(): void {
|
||||||
|
// PROTECTION: Atomic check-and-set to prevent race conditions
|
||||||
|
// Flag is set IMMEDIATELY after check, BEFORE any operations
|
||||||
|
// This prevents:
|
||||||
|
// 1. Multiple calls to destroy() (reentrancy guard)
|
||||||
|
// 2. Operations from executing after destroy() starts
|
||||||
|
// 3. New queues from being created after cleanup
|
||||||
|
if (this.destroyed) {
|
||||||
|
this.logger.debug('PreKeyManager already destroyed')
|
||||||
|
return
|
||||||
|
}
|
||||||
|
this.destroyed = true // ← Set IMMEDIATELY to close race window
|
||||||
|
|
||||||
this.logger.debug('🗑️ Destroying PreKeyManager')
|
this.logger.debug('🗑️ Destroying PreKeyManager')
|
||||||
|
|
||||||
this.queues.forEach((queue, keyType) => {
|
this.queues.forEach((queue, keyType) => {
|
||||||
|
|||||||
Reference in New Issue
Block a user