From 3ebfcec7db84540b6043997187575348dcd5d8de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20Moy=C3=A8re?= Date: Wed, 24 Jan 2024 11:56:30 +0100 Subject: [PATCH 1/4] Fix a race condition where a call to GetTenantScope was preventing ReconfigureTenant to set the new tenant scope instance. --- src/Autofac.Multitenant/MultitenantContainer.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Autofac.Multitenant/MultitenantContainer.cs b/src/Autofac.Multitenant/MultitenantContainer.cs index e1b5990..1d64c78 100644 --- a/src/Autofac.Multitenant/MultitenantContainer.cs +++ b/src/Autofac.Multitenant/MultitenantContainer.cs @@ -342,7 +342,7 @@ public void ConfigureTenant(object tenantId, Action configurat /// This should happen very rarely, hopefully never. /// [SuppressMessage("CA1513", "CA1513", Justification = "ObjectDisposedException.ThrowIf is not available in all target frameworks.")] - private ILifetimeScope CreateTenantScope(object tenantId, Action? configuration = null) + private ILifetimeScope CreateTenantScope(object tenantId, Action? configuration = null, bool forced = false) { if (_isDisposed == 1) { @@ -353,7 +353,17 @@ private ILifetimeScope CreateTenantScope(object tenantId, Action configur removed = true; } - CreateTenantScope(tenantId, configuration); + CreateTenantScope(tenantId, configuration, true); return removed; } From 59ddb5e1018eb7fbb4702e8a069440c05e0a0574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20Moy=C3=A8re?= Date: Wed, 24 Jan 2024 18:40:48 +0100 Subject: [PATCH 2/4] Enhance ReconfigureTenant with atomic changes to let GetTenantScope run in parallel if needed (in the previous solution, calls to GetTenantScope could return an empty scope until the call the ReconfigureTenant is completed) --- .../MultitenantContainer.cs | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/src/Autofac.Multitenant/MultitenantContainer.cs b/src/Autofac.Multitenant/MultitenantContainer.cs index 1d64c78..8eda896 100644 --- a/src/Autofac.Multitenant/MultitenantContainer.cs +++ b/src/Autofac.Multitenant/MultitenantContainer.cs @@ -404,6 +404,7 @@ private ILifetimeScope CreateTenantScope(object tenantId, Actiontrue if an existing configuration was removed; otherwise, false. /// /// + [SuppressMessage("CA1513", "CA1513", Justification = "ObjectDisposedException.ThrowIf is not available in all target frameworks.")] public bool ReconfigureTenant(object tenantId, Action configuration) { if (configuration == null) @@ -411,18 +412,35 @@ public bool ReconfigureTenant(object tenantId, Action configur throw new ArgumentNullException(nameof(configuration)); } + if (_isDisposed == 1) + { + throw new ObjectDisposedException(nameof(ApplicationContainer)); + } + tenantId ??= _defaultTenantId; - var removed = false; - if (_tenantLifetimeScopes.TryRemove(tenantId, out var tenantScope)) + var lifetimeScope = ApplicationContainer.BeginLifetimeScope(TenantLifetimeScopeTag, configuration); + + ILifetimeScope? disposedLifetimeScope = null; + _tenantLifetimeScopes.AddOrUpdate( + tenantId, + _ => lifetimeScope, + (_, updatedLifetimeScope) => + { + // We defer the existing scope Dispose() + // as we prefer enabling the new scope as quickly + // as possible. + disposedLifetimeScope = updatedLifetimeScope; + return lifetimeScope; + }); + + if (disposedLifetimeScope == null) { - tenantScope.Dispose(); - removed = true; + return false; } - CreateTenantScope(tenantId, configuration, true); - - return removed; + disposedLifetimeScope.Dispose(); + return true; } /// From 9ec08527929b4392e6c1867b5b9452121004ef95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicolas=20Moy=C3=A8re?= Date: Thu, 25 Jan 2024 17:13:10 +0100 Subject: [PATCH 3/4] Revert the CreateTenantScope changes as they are useless with the new ReconfigureTenant implementation --- src/Autofac.Multitenant/MultitenantContainer.cs | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/Autofac.Multitenant/MultitenantContainer.cs b/src/Autofac.Multitenant/MultitenantContainer.cs index 8eda896..aa05932 100644 --- a/src/Autofac.Multitenant/MultitenantContainer.cs +++ b/src/Autofac.Multitenant/MultitenantContainer.cs @@ -342,7 +342,7 @@ public void ConfigureTenant(object tenantId, Action configurat /// This should happen very rarely, hopefully never. /// [SuppressMessage("CA1513", "CA1513", Justification = "ObjectDisposedException.ThrowIf is not available in all target frameworks.")] - private ILifetimeScope CreateTenantScope(object tenantId, Action? configuration = null, bool forced = false) + private ILifetimeScope CreateTenantScope(object tenantId, Action? configuration = null) { if (_isDisposed == 1) { @@ -353,17 +353,7 @@ private ILifetimeScope CreateTenantScope(object tenantId, Action Date: Thu, 25 Jan 2024 17:13:24 +0100 Subject: [PATCH 4/4] Add new unit tests --- .../MultitenantContainerFixture.cs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/Autofac.Multitenant.Test/MultitenantContainerFixture.cs b/test/Autofac.Multitenant.Test/MultitenantContainerFixture.cs index ee485c6..cca0762 100644 --- a/test/Autofac.Multitenant.Test/MultitenantContainerFixture.cs +++ b/test/Autofac.Multitenant.Test/MultitenantContainerFixture.cs @@ -567,6 +567,52 @@ public void ReconfigureTenant_ReconfigureSingleton() Assert.IsType(mtc.Resolve()); } + [Fact] + public void ReconfigureTenant_RequiresConfiguration() + { + var builder = new ContainerBuilder(); + var strategy = new StubTenantIdentificationStrategy() + { + TenantId = "tenant1", + }; + using var mtc = new MultitenantContainer(strategy, builder.Build()); + mtc.ConfigureTenant("tenant1", b => b.RegisterType().AsImplementedInterfaces().SingleInstance()); + + Assert.Throws(() => mtc.ReconfigureTenant("tenant1", null)); + } + + [Fact] + public void ReconfigureTenant_ThrowsAfterDisposal() + { + var builder = new ContainerBuilder(); + var strategy = new StubTenantIdentificationStrategy() + { + TenantId = "tenant1", + }; + using var mtc = new MultitenantContainer(strategy, builder.Build()); + mtc.Dispose(); + Assert.Throws(() => mtc.ReconfigureTenant("tenant1", _ => { })); + } + + [Fact] + public void ReconfigureTenant_AddLifetimeScope() + { + var builder = new ContainerBuilder(); + var strategy = new StubTenantIdentificationStrategy() + { + TenantId = "tenant1", + }; + using var mtc = new MultitenantContainer(strategy, builder.Build()); + mtc.ConfigureTenant("tenant1", b => b.RegisterType().AsImplementedInterfaces().SingleInstance()); + + // Simulate another thread playing with the MTC + mtc.RemoveTenant("tenant1"); + + mtc.ReconfigureTenant("tenant1", b => b.RegisterType().AsImplementedInterfaces().SingleInstance()); + + Assert.IsType(mtc.Resolve()); + } + [Fact] public void GetTenants_CheckRegistered() {