Skip to content

Commit 722c141

Browse files
author
Marvin Zhang
committed
refactor: enhance initialization process in ProjectDevlogManager and TypeORMStorageProvider to prevent race conditions
1 parent 9a4d854 commit 722c141

File tree

3 files changed

+153
-9
lines changed

3 files changed

+153
-9
lines changed

packages/core/src/managers/devlog/project-devlog-manager.ts

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,28 +39,57 @@ export class ProjectDevlogManager {
3939

4040
/**
4141
* Initialize the devlog manager
42+
* Protects against race conditions during concurrent initialization
4243
*/
4344
async initialize(): Promise<void> {
45+
// If already initialized, return immediately
4446
if (this.initialized) {
4547
console.log('🔄 ProjectDevlogManager already initialized, skipping...');
4648
return;
4749
}
4850

51+
// If initialization is in progress, wait for it
52+
if (this.initPromise) {
53+
console.log('⏳ ProjectDevlogManager initialization in progress, waiting...');
54+
return this.initPromise;
55+
}
56+
4957
console.log('🚀 Initializing ProjectDevlogManager...');
5058

51-
if (this.options.projectContext) {
52-
console.log(`📂 Project context: ${this.options.projectContext.projectId}`);
53-
} else {
54-
console.log('📂 No project context configured');
59+
// Create initialization promise to prevent race conditions
60+
this.initPromise = this.performInitialization();
61+
62+
try {
63+
await this.initPromise;
64+
} catch (error) {
65+
// Clear promise on failure so next call can retry
66+
this.initPromise = null;
67+
throw error;
5568
}
69+
}
5670

57-
console.log(`💾 Storage type: ${this.options.storageConfig.type}`);
71+
/**
72+
* Internal method to perform the actual initialization
73+
*/
74+
private async performInitialization(): Promise<void> {
75+
try {
76+
if (this.options.projectContext) {
77+
console.log(`📂 Project context: ${this.options.projectContext.projectId}`);
78+
} else {
79+
console.log('📂 No project context configured');
80+
}
5881

59-
this.storageProvider = await StorageProviderFactory.create(this.options.storageConfig);
60-
await this.storageProvider.initialize();
82+
console.log(`💾 Storage type: ${this.options.storageConfig.type}`);
6183

62-
this.initialized = true;
63-
console.log('✅ ProjectDevlogManager initialized successfully');
84+
this.storageProvider = await StorageProviderFactory.create(this.options.storageConfig);
85+
await this.storageProvider.initialize();
86+
87+
this.initialized = true;
88+
console.log('✅ ProjectDevlogManager initialized successfully');
89+
} catch (error) {
90+
console.error('❌ ProjectDevlogManager initialization failed:', error);
91+
throw error;
92+
}
6493
}
6594

6695
/**

packages/core/src/storage/providers/typeorm-storage.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ export class TypeORMStorageProvider implements StorageProvider {
4949
private chatDevlogLinkRepository?: Repository<ChatDevlogLinkEntity>;
5050
private options: TypeORMStorageOptions;
5151

52+
// Initialization race condition protection
53+
private initializationPromise: Promise<void> | null = null;
54+
5255
// Event subscription properties
5356
private eventCallbacks = new Set<(event: DevlogEvent) => void>();
5457
private isWatching = false;
@@ -59,9 +62,37 @@ export class TypeORMStorageProvider implements StorageProvider {
5962
}
6063

6164
async initialize(): Promise<void> {
65+
// If already initialized, return immediately
66+
if (this.dataSource.isInitialized && this.repository) {
67+
console.log('[TypeORMStorage] Already initialized, skipping...');
68+
return;
69+
}
70+
71+
// If initialization is in progress, wait for it
72+
if (this.initializationPromise) {
73+
console.log('[TypeORMStorage] Initialization in progress, waiting...');
74+
return this.initializationPromise;
75+
}
76+
77+
console.log('[TypeORMStorage] Starting new initialization...');
78+
79+
// Create initialization promise to prevent race conditions
80+
this.initializationPromise = this.doInitialize();
81+
82+
try {
83+
await this.initializationPromise;
84+
} catch (error) {
85+
// Clear the promise on failure so next call can retry
86+
this.initializationPromise = null;
87+
throw error;
88+
}
89+
}
90+
91+
private async doInitialize(): Promise<void> {
6292
try {
6393
// Check if already initialized to prevent "already connected" errors in development
6494
if (!this.dataSource.isInitialized) {
95+
console.log(`[TypeORMStorage] Connecting to ${this.options.type} database...`);
6596
await this.dataSource.initialize();
6697
console.log('[TypeORMStorage] Connected to database successfully');
6798
} else {
@@ -84,7 +115,10 @@ export class TypeORMStorageProvider implements StorageProvider {
84115
// Don't fail initialization if chat tables already exist
85116
}
86117
}
118+
119+
console.log('[TypeORMStorage] Repositories initialized successfully');
87120
} catch (error) {
121+
console.error('[TypeORMStorage] Initialization failed:', error);
88122
throw new Error(`Failed to initialize TypeORM storage: ${error}`);
89123
}
90124
}

packages/core/src/storage/storage-provider.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,42 @@ import { TypeORMStorageOptions } from './typeorm/typeorm-config.js';
88

99
/**
1010
* Factory for creating storage providers based on configuration
11+
* Implements singleton pattern to prevent race conditions and duplicate connections
1112
*/
1213
export class StorageProviderFactory {
14+
private static instanceCache = new Map<string, Promise<StorageProvider>>();
15+
1316
static async create(config?: StorageConfig): Promise<StorageProvider> {
17+
// Generate cache key based on configuration
18+
const cacheKey = StorageProviderFactory.generateCacheKey(config);
19+
20+
// Return existing instance if available
21+
if (StorageProviderFactory.instanceCache.has(cacheKey)) {
22+
console.log(`📦 StorageProviderFactory: Returning cached ${config?.type} provider`);
23+
return StorageProviderFactory.instanceCache.get(cacheKey)!;
24+
}
25+
26+
console.log(`🚀 StorageProviderFactory: Creating new ${config?.type} provider`);
27+
28+
// Create initialization promise to prevent race conditions
29+
const initPromise = StorageProviderFactory.createProvider(config);
30+
31+
// Cache the promise immediately to prevent concurrent creation
32+
StorageProviderFactory.instanceCache.set(cacheKey, initPromise);
33+
34+
try {
35+
const provider = await initPromise;
36+
console.log(`✅ StorageProviderFactory: ${config?.type} provider created and initialized`);
37+
return provider;
38+
} catch (error) {
39+
// Remove failed initialization from cache so it can be retried
40+
StorageProviderFactory.instanceCache.delete(cacheKey);
41+
console.error(`❌ StorageProviderFactory: Failed to create ${config?.type} provider:`, error);
42+
throw error;
43+
}
44+
}
45+
46+
private static async createProvider(config?: StorageConfig): Promise<StorageProvider> {
1447
// Handle storage types (GitHub is now an integration, not storage)
1548
switch (config?.type) {
1649
case 'sqlite':
@@ -78,4 +111,52 @@ export class StorageProviderFactory {
78111
return new JsonStorageProvider(config?.json || {});
79112
}
80113
}
114+
115+
/**
116+
* Generate cache key based on storage configuration
117+
*/
118+
private static generateCacheKey(config?: StorageConfig): string {
119+
if (!config) {
120+
return 'json:default';
121+
}
122+
123+
switch (config.type) {
124+
case 'sqlite':
125+
return `sqlite:${config.connectionString || '.devlog/devlog.sqlite'}`;
126+
127+
case 'postgres':
128+
return `postgres:${config.connectionString || 'default'}`;
129+
130+
case 'mysql':
131+
if (config.connectionString) {
132+
return `mysql:${config.connectionString}`;
133+
}
134+
const host = config.options?.host || 'localhost';
135+
const port = config.options?.port || 3306;
136+
const database = config.options?.database || 'default';
137+
return `mysql:${host}:${port}:${database}`;
138+
139+
case 'json':
140+
default:
141+
return 'json:default';
142+
}
143+
}
144+
145+
/**
146+
* Clear the instance cache (useful for testing or configuration changes)
147+
*/
148+
static clearCache(): void {
149+
console.log('🧹 StorageProviderFactory: Clearing instance cache');
150+
StorageProviderFactory.instanceCache.clear();
151+
}
152+
153+
/**
154+
* Get cache statistics for debugging
155+
*/
156+
static getCacheStats(): { size: number; keys: string[] } {
157+
return {
158+
size: StorageProviderFactory.instanceCache.size,
159+
keys: Array.from(StorageProviderFactory.instanceCache.keys()),
160+
};
161+
}
81162
}

0 commit comments

Comments
 (0)