Skip to content

Fix static field mutation in CrystaProgramSyncModuleService constructor#115

Merged
afshinalizadeh merged 6 commits intofeature/afshin/add-sync-githubfrom
copilot/sub-pr-113-again
Jan 8, 2026
Merged

Fix static field mutation in CrystaProgramSyncModuleService constructor#115
afshinalizadeh merged 6 commits intofeature/afshin/add-sync-githubfrom
copilot/sub-pr-113-again

Conversation

Copy link
Contributor

Copilot AI commented Dec 26, 2025

The service constructor was writing to a static field, causing thread-safety issues and defeating transient DI registration. Multiple service instances shared the same static _modules list, leading to race conditions.

Changes:

  • Converted to singleton with factory pattern: Changed from AppDbContext injection to IDbContextFactory<AppDbContext>, enabling singleton lifetime with scoped DbContext creation
  • Removed static fields: Made _modules instance-based with thread-safe lazy initialization using SemaphoreSlim
  • Added proper disposal: Implemented IDisposable to cleanup semaphore resources
  • Fixed cancellation propagation: Added CancellationToken parameter to UpdateSyncModuleAsync interface and propagated through all async operations

Before:

public class CrystaProgramSyncModuleService {
    private static List<CrystaProgramSyncModule> _modules = new();
    
    public CrystaProgramSyncModuleService(AppDbContext dbContext) {
        if (_modules.Count == 0) {
            _modules = dbContext.Set<CrystaProgramSyncModule>()...;  // ❌ Static mutation
        }
    }
}

services.AddTransient<ICrystaProgramSyncModuleService, ...>();  // ❌ Wrong lifetime

After:

public class CrystaProgramSyncModuleService : IDisposable {
    private List<CrystaProgramSyncModule> _modules = new();
    private readonly SemaphoreSlim _initLock = new(1, 1);
    
    public CrystaProgramSyncModuleService(IDbContextFactory<AppDbContext> factory) {
        this.DbContextFactory = factory;
    }
    
    private async Task EnsureInitializedAsync(CancellationToken ct) {
        await _initLock.WaitAsync(ct);
        try {
            if (!_initialized) {
                await using var db = await DbContextFactory.CreateDbContextAsync(ct);
                _modules = await db.Set<CrystaProgramSyncModule>()...;
            }
        } finally { _initLock.Release(); }
    }
}

services.AddSingleton<ICrystaProgramSyncModuleService, ...>();  // ✓ Correct lifetime

Applied same pattern to CrystaProgramSyncModuleServiceFake.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 5 commits December 26, 2025 21:05
- Removed static keyword from _modules field to prevent shared state issues
- Changed from AppDbContext to IDbContextFactory<AppDbContext> for proper singleton support
- Added lazy initialization with thread-safe semaphore lock
- Changed service registration from Transient to Singleton
- Updated Fake service with same pattern for consistency

Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>
- Add Microsoft.EntityFrameworkCore using statement for EF Core extensions
- Add thread-safe synchronization for UpdateSyncModuleAsync with SemaphoreSlim
- Move initialization to async method with proper thread-safety in Fake service
- Add synchronization locks for in-memory list modifications

Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>
- Add CancellationToken parameter to interface method with default value
- Propagate cancellation token through all database operations
- Pass cancellation token to AddAsync and SaveChangesAsync methods

Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>
- Add IDisposable implementation to both service classes
- Properly dispose SemaphoreSlim instances to free unmanaged resources
- Add _disposed flag to prevent double disposal

Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>
- Pass cancellation token to semaphore wait operations
- Enable proper cancellation propagation during initialization

Co-authored-by: afshinalizadeh <4254006+afshinalizadeh@users.noreply.github.com>
Copilot AI changed the title [WIP] Update sync GitHub feature implementation based on feedback Fix static field mutation in CrystaProgramSyncModuleService constructor Dec 26, 2025
Copilot AI requested a review from afshinalizadeh December 26, 2025 21:16
@afshinalizadeh afshinalizadeh marked this pull request as ready for review January 8, 2026 15:33
@afshinalizadeh afshinalizadeh merged commit dd2de65 into feature/afshin/add-sync-github Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants