diff --git a/src/Business/Grand.Business.Storage/Services/AmazonPictureService.cs b/src/Business/Grand.Business.Storage/Services/AmazonPictureService.cs index eedd296ce..a9ffb7d10 100644 --- a/src/Business/Grand.Business.Storage/Services/AmazonPictureService.cs +++ b/src/Business/Grand.Business.Storage/Services/AmazonPictureService.cs @@ -185,9 +185,9 @@ protected override string GetThumbUrl(string thumbFileName, string storeLocation /// /// Thumb file name /// Picture binary - protected override Task SaveThumb(string thumbFileName, byte[] binary) + protected override async Task SaveThumb(string thumbFileName, byte[] binary) { - CheckBucketExists().Wait(); + await CheckBucketExists(); using (Stream stream = new MemoryStream(binary)) { @@ -197,11 +197,10 @@ protected override Task SaveThumb(string thumbFileName, byte[] binary) Key = thumbFileName, StorageClass = S3StorageClass.Standard }; - _s3Client.PutObjectAsync(putObjectRequest).Wait(); + await _s3Client.PutObjectAsync(putObjectRequest); } - _s3Client.MakeObjectPublicAsync(_bucketName, thumbFileName, true).Wait(); - return Task.CompletedTask; + await _s3Client.MakeObjectPublicAsync(_bucketName, thumbFileName, true); } /// diff --git a/src/Core/Grand.Infrastructure/Caching/MemoryCacheBase.cs b/src/Core/Grand.Infrastructure/Caching/MemoryCacheBase.cs index ce61ca8d2..a328ed801 100644 --- a/src/Core/Grand.Infrastructure/Caching/MemoryCacheBase.cs +++ b/src/Core/Grand.Infrastructure/Caching/MemoryCacheBase.cs @@ -110,25 +110,21 @@ public virtual async Task SetAsync(string key, Func> acquire, int } } - public virtual Task RemoveAsync(string key, bool publisher = true) + public virtual async Task RemoveAsync(string key, bool publisher = true) { _cache.Remove(key); if (publisher) - _mediator.Publish(new EntityCacheEvent(key, CacheEvent.RemoveKey)); - - return Task.CompletedTask; + await _mediator.Publish(new EntityCacheEvent(key, CacheEvent.RemoveKey)); } - public virtual Task RemoveByPrefix(string prefix, bool publisher = true) + public virtual async Task RemoveByPrefix(string prefix, bool publisher = true) { var entriesToRemove = CacheEntries.Where(x => x.Key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)); foreach (var cacheEntries in entriesToRemove) _cache.Remove(cacheEntries.Key); if (publisher) - _mediator.Publish(new EntityCacheEvent(prefix, CacheEvent.RemovePrefix)); - - return Task.CompletedTask; + await _mediator.Publish(new EntityCacheEvent(prefix, CacheEvent.RemovePrefix)); } public virtual Task Clear(bool publisher = true) @@ -137,10 +133,11 @@ public virtual Task Clear(bool publisher = true) foreach (var cacheEntry in CacheEntries.Keys.ToList()) _cache.Remove(cacheEntry); - //cancel + //cancel, but do not dispose: a writer that already read this source is still handing it to + //MemoryCache, which registers an eviction callback on it while storing the entry, and that + //registration throws ObjectDisposedException on a disposed source. Cancelling releases the + //registrations, and the source has no timer and no WaitHandle, so it is simply collected _resetCacheToken.Cancel(); - //dispose - _resetCacheToken.Dispose(); _resetCacheToken = new CancellationTokenSource(); diff --git a/src/Core/Grand.Infrastructure/StartupBase.cs b/src/Core/Grand.Infrastructure/StartupBase.cs index 3c279689f..8bee26d0b 100644 --- a/src/Core/Grand.Infrastructure/StartupBase.cs +++ b/src/Core/Grand.Infrastructure/StartupBase.cs @@ -231,7 +231,8 @@ private static void RegisterConfigurations(IServiceCollection services, IConfigu /// /// Collection of service descriptors /// Configuration root of the application - public static void ConfigureServices(IServiceCollection services, IConfiguration configuration) + public static void ConfigureServices(IServiceCollection services, IConfiguration configuration, + IWebHostEnvironment hostingEnvironment) { services.AddFeatureManagement(); @@ -239,26 +240,21 @@ public static void ConfigureServices(IServiceCollection services, IConfiguration var typeSearcher = new TypeSearcher(); services.AddSingleton(typeSearcher); - var provider = services.BuildServiceProvider(); - var hostingEnvironment = provider.GetRequiredService(); - //register application var mvcBuilder = RegisterApplication(services, configuration, hostingEnvironment, typeSearcher); - //register extensions + //register extensions RegisterExtensions(mvcBuilder, configuration, hostingEnvironment); - var startupConfigurations = typeSearcher.ClassesOfType(); - - //Register startup - var instancesBefore = startupConfigurations + //instantiate once - the same instances serve both configuration passes + var startupInstances = typeSearcher.ClassesOfType() .Where(PluginExtensions.OnlyInstalledPlugins) .Select(startup => (IStartupApplication)Activator.CreateInstance(startup)) - .Where(startup => startup!.BeforeConfigure) - .OrderBy(startup => startup.Priority); + .OrderBy(startup => startup!.Priority) + .ToList(); //configure services - foreach (var instance in instancesBefore) + foreach (var instance in startupInstances.Where(startup => startup.BeforeConfigure)) instance.ConfigureServices(services, configuration); //register mapper configurations @@ -273,15 +269,8 @@ public static void ConfigureServices(IServiceCollection services, IConfiguration //add mediator AddMediator(services, typeSearcher); - //Register startup - var instancesAfter = startupConfigurations - .Where(PluginExtensions.OnlyInstalledPlugins) - .Select(startup => (IStartupApplication)Activator.CreateInstance(startup)) - .Where(startup => !startup!.BeforeConfigure) - .OrderBy(startup => startup.Priority); - //configure services - foreach (var instance in instancesAfter) + foreach (var instance in startupInstances.Where(startup => !startup.BeforeConfigure)) instance.ConfigureServices(services, configuration); //Execute startup interface diff --git a/src/Modules/Grand.Module.Migration/Startup/StartupApplication.cs b/src/Modules/Grand.Module.Migration/Startup/StartupApplication.cs index 4121094c0..5f9d80e2d 100644 --- a/src/Modules/Grand.Module.Migration/Startup/StartupApplication.cs +++ b/src/Modules/Grand.Module.Migration/Startup/StartupApplication.cs @@ -22,11 +22,15 @@ public void Configure(WebApplication application, IWebHostEnvironment webHostEnv if (!DataSettingsManager.DatabaseIsInstalled()) return; var featureManager = application.Services.GetRequiredService(); - if (featureManager.IsEnabledAsync("Grand.Module.Migration").Result) - { - var migrationProcess = application.Services.GetRequiredService(); - migrationProcess.RunMigrationProcess(); - } + //Configure is synchronous, so the startup path has to block here + if (!featureManager.IsEnabledAsync("Grand.Module.Migration").GetAwaiter().GetResult()) + return; + + //IMigrationProcess is scoped - resolving it from the root provider fails scope validation + //and otherwise roots the repository graph for the lifetime of the process + using var scope = application.Services.CreateScope(); + var migrationProcess = scope.ServiceProvider.GetRequiredService(); + migrationProcess.RunMigrationProcess(); } public int Priority => 100; diff --git a/src/Tests/Grand.Infrastructure.Tests/Caching/MemoryCacheBaseTests.cs b/src/Tests/Grand.Infrastructure.Tests/Caching/MemoryCacheBaseTests.cs index 475ed1f93..78943c1ee 100644 --- a/src/Tests/Grand.Infrastructure.Tests/Caching/MemoryCacheBaseTests.cs +++ b/src/Tests/Grand.Infrastructure.Tests/Caching/MemoryCacheBaseTests.cs @@ -1,5 +1,6 @@ using Grand.Infrastructure.Caching; using Grand.Infrastructure.Configuration; +using Grand.Infrastructure.Events; using Grand.Mediator; using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.DependencyInjection; @@ -139,4 +140,66 @@ public async Task SetAsync_Key_Exist_ShouldSetCacheEntry() Assert.IsNotNull(cacheResult); Assert.AreEqual(cacheEntry, cacheResult); } + + [TestMethod] + public async Task RemoveAsync_AwaitsTheNotification() + { + _mediatorMock + .Setup(x => x.Publish(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("handler failed")); + + //a fire-and-forget Publish would swallow this + await Assert.ThrowsAsync(() => _service.RemoveAsync("key")); + } + + [TestMethod] + public async Task RemoveByPrefix_AwaitsTheNotification() + { + _mediatorMock + .Setup(x => x.Publish(It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("handler failed")); + + await Assert.ThrowsAsync(() => _service.RemoveByPrefix("key")); + } + + /// + /// Guards against disposing the reset token in . + /// + /// + /// A writer reads the token source, then MemoryCache registers an eviction callback on it while + /// storing the entry. Disposing the previous source in Clear makes that registration throw + /// ObjectDisposedException, and swapping the field first only narrows the window rather than + /// closing it. The assertion only fires on an exception actually raised by the race, so this + /// cannot fail spuriously - it can only miss. + /// + [TestMethod] + [Timeout(60000)] + [DoNotParallelize] + public void Clear_WhileEntriesAreBeingWritten_DoesNotThrow() + { + Exception captured = null; + var stopWriting = false; + + var writer = Task.Run(async () => + { + var i = 0; + while (!stopWriting) + try + { + await _service.SetAsync($"race-{i++}", () => Task.FromResult("value")); + } + catch (Exception ex) + { + captured ??= ex; + return; + } + }); + + for (var i = 0; i < 5000 && captured == null; i++) _service.Clear(false).GetAwaiter().GetResult(); + + stopWriting = true; + writer.Wait(TimeSpan.FromSeconds(5)); + + Assert.IsNull(captured, $"Clear raced a concurrent write: {captured}"); + } } \ No newline at end of file diff --git a/src/Web/Grand.Web.Admin/Program.cs b/src/Web/Grand.Web.Admin/Program.cs index 7c7c724ba..566d27e39 100644 --- a/src/Web/Grand.Web.Admin/Program.cs +++ b/src/Web/Grand.Web.Admin/Program.cs @@ -15,7 +15,7 @@ }); //add services -StartupBase.ConfigureServices(builder.Services, builder.Configuration); +StartupBase.ConfigureServices(builder.Services, builder.Configuration, builder.Environment); builder.ConfigureApplicationSettings(); diff --git a/src/Web/Grand.Web.Store/Program.cs b/src/Web/Grand.Web.Store/Program.cs index 7c7c724ba..566d27e39 100644 --- a/src/Web/Grand.Web.Store/Program.cs +++ b/src/Web/Grand.Web.Store/Program.cs @@ -15,7 +15,7 @@ }); //add services -StartupBase.ConfigureServices(builder.Services, builder.Configuration); +StartupBase.ConfigureServices(builder.Services, builder.Configuration, builder.Environment); builder.ConfigureApplicationSettings(); diff --git a/src/Web/Grand.Web.Vendor/Program.cs b/src/Web/Grand.Web.Vendor/Program.cs index 0ec1da027..ba31a8c97 100644 --- a/src/Web/Grand.Web.Vendor/Program.cs +++ b/src/Web/Grand.Web.Vendor/Program.cs @@ -15,7 +15,7 @@ }); //add services -StartupBase.ConfigureServices(builder.Services, builder.Configuration); +StartupBase.ConfigureServices(builder.Services, builder.Configuration, builder.Environment); builder.ConfigureApplicationSettings(); diff --git a/src/Web/Grand.Web/Program.cs b/src/Web/Grand.Web/Program.cs index e9e212289..973f4bc23 100644 --- a/src/Web/Grand.Web/Program.cs +++ b/src/Web/Grand.Web/Program.cs @@ -16,7 +16,7 @@ }); //add services -StartupBase.ConfigureServices(builder.Services, builder.Configuration); +StartupBase.ConfigureServices(builder.Services, builder.Configuration, builder.Environment); builder.ConfigureApplicationSettings();