From a9c8ad64f1ad388487e477cc2ff497d74ce44f4c Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Thu, 12 Dec 2024 11:20:45 +0100 Subject: [PATCH] Add concurrency locks to DefaultTenantService operations Introduced SemaphoreSlim for initialization and refresh methods to ensure thread safety in DefaultTenantService. Improved tenant unregistration to handle scope cleanup only when mappings exist. These changes enhance reliability and prevent race conditions during tenant operations. --- .../Implementations/DefaultTenantService.cs | 79 ++++++++++++------- 1 file changed, 52 insertions(+), 27 deletions(-) diff --git a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs index 9db13dc36..fca84ebc9 100644 --- a/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs +++ b/src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantService.cs @@ -5,12 +5,15 @@ namespace Elsa.Common.Multitenancy; public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScopeFactory tenantScopeFactory, TenantEventsManager tenantEvents, ITenantAccessor tenantAccessor) : ITenantService, IAsyncDisposable { private readonly AsyncServiceScope _serviceScope = scopeFactory.CreateAsyncScope(); + private readonly SemaphoreSlim _initializationLock = new(1, 1); + private readonly SemaphoreSlim _refreshLock = new(1, 1); private IDictionary? _tenantsDictionary; private IDictionary? _tenantScopesDictionary; public async ValueTask DisposeAsync() { await _serviceScope.DisposeAsync(); + _initializationLock.Dispose(); } public async Task FindAsync(string id, CancellationToken cancellationToken = default) @@ -56,7 +59,8 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop public async Task DeactivateTenantsAsync(CancellationToken cancellationToken = default) { - var tenants = _tenantsDictionary!.Values.ToArray(); + var dictionary = await GetTenantsDictionaryAsync(cancellationToken); + var tenants = dictionary.Values.ToArray(); foreach (var tenant in tenants) await UnregisterTenantAsync(tenant, cancellationToken); @@ -64,25 +68,34 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop public async Task RefreshAsync(CancellationToken cancellationToken = default) { - await using var scope = scopeFactory.CreateAsyncScope(); - var tenantsProvider = scope.ServiceProvider.GetRequiredService(); - var currentTenants = await GetTenantsDictionaryAsync(cancellationToken); - var currentTenantIds = currentTenants.Keys; - var newTenants = (await tenantsProvider.ListAsync(cancellationToken)).ToDictionary(x => x.Id); - var newTenantIds = newTenants.Keys; - var removedTenantIds = currentTenantIds.Except(newTenantIds).ToArray(); - var addedTenantIds = newTenantIds.Except(currentTenantIds).ToArray(); + await _refreshLock.WaitAsync(cancellationToken); - foreach (var removedTenantId in removedTenantIds) + try { - var removedTenant = currentTenants[removedTenantId]; - await UnregisterTenantAsync(removedTenant, cancellationToken); + await using var scope = scopeFactory.CreateAsyncScope(); + var tenantsProvider = scope.ServiceProvider.GetRequiredService(); + var currentTenants = await GetTenantsDictionaryAsync(cancellationToken); + var currentTenantIds = currentTenants.Keys; + var newTenants = (await tenantsProvider.ListAsync(cancellationToken)).ToDictionary(x => x.Id); + var newTenantIds = newTenants.Keys; + var removedTenantIds = currentTenantIds.Except(newTenantIds).ToArray(); + var addedTenantIds = newTenantIds.Except(currentTenantIds).ToArray(); + + foreach (var removedTenantId in removedTenantIds) + { + var removedTenant = currentTenants[removedTenantId]; + await UnregisterTenantAsync(removedTenant, cancellationToken); + } + + foreach (var addedTenantId in addedTenantIds) + { + var addedTenant = newTenants[addedTenantId]; + await RegisterTenantAsync(addedTenant, cancellationToken); + } } - - foreach (var addedTenantId in addedTenantIds) + finally { - var addedTenant = newTenants[addedTenantId]; - await RegisterTenantAsync(addedTenant, cancellationToken); + _refreshLock.Release(); } } @@ -90,13 +103,24 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop { if (_tenantsDictionary == null) { - _tenantsDictionary = new Dictionary(); - _tenantScopesDictionary = new Dictionary(); - var tenantsProvider = _serviceScope.ServiceProvider.GetRequiredService(); - var tenants = await tenantsProvider.ListAsync(cancellationToken); + await _initializationLock.WaitAsync(cancellationToken); // Lock to ensure single-threaded initialization + try + { + if (_tenantsDictionary == null) // Double-check locking + { + _tenantsDictionary = new Dictionary(); + _tenantScopesDictionary = new Dictionary(); + var tenantsProvider = _serviceScope.ServiceProvider.GetRequiredService(); + var tenants = await tenantsProvider.ListAsync(cancellationToken); - foreach (var tenant in tenants) - await RegisterTenantAsync(tenant, cancellationToken); + foreach (var tenant in tenants) + await RegisterTenantAsync(tenant, cancellationToken); + } + } + finally + { + _initializationLock.Release(); + } } return _tenantsDictionary; @@ -114,11 +138,12 @@ public class DefaultTenantService(IServiceScopeFactory scopeFactory, ITenantScop private async Task UnregisterTenantAsync(Tenant tenant, CancellationToken cancellationToken = default) { - var scope = _tenantScopesDictionary![tenant]; - _tenantsDictionary!.Remove(tenant.Id); - _tenantScopesDictionary!.Remove(tenant); + if (_tenantScopesDictionary!.Remove(tenant, out var scope)) + { + _tenantsDictionary!.Remove(tenant.Id, out _); - using (tenantAccessor.PushContext(tenant)) - await tenantEvents.TenantDeactivatedAsync(new TenantDeactivatedEventArgs(tenant, scope, cancellationToken)); + using (tenantAccessor.PushContext(tenant)) + await tenantEvents.TenantDeactivatedAsync(new TenantDeactivatedEventArgs(tenant, scope, cancellationToken)); + } } } \ No newline at end of file