fix infinite loop in Token Cleanup after concurrency exception #4492 (#…

…4498)
IdentityServer · Jun 11, 2020 · 3fef0b0 · 3fef0b0
1 parent c67699c
commit 3fef0b0
Show file tree

Hide file tree

Showing 4 changed files with 108 additions and 25 deletions.
diff --git a/src/EntityFramework.Storage/IdentityServer4.EntityFramework.Storage.sln b/src/EntityFramework.Storage/IdentityServer4.EntityFramework.Storage.sln
@@ -1,7 +1,7 @@
 
 Microsoft Visual Studio Solution File, Format Version 12.00
-# Visual Studio 15
-VisualStudioVersion = 15.0.26124.0
+# Visual Studio Version 16
+VisualStudioVersion = 16.0.30204.135
 MinimumVisualStudioVersion = 15.0.26124.0
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{AF5DAC33-08AC-45EE-9772-4FF39FB09E57}"
 EndProject
@@ -17,6 +17,10 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "migrations", "migrations",
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "SqlServer", "migrations\SqlServer\SqlServer.csproj", "{A93A59EC-D75D-44E1-9720-F75D9EF95BC3}"
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "host", "host", "{07C56E10-A807-4372-ACD9-ADED2D099BC8}"
+EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ConsoleHost", "host\ConsoleHost\ConsoleHost.csproj", "{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -75,6 +79,18 @@ Global
 		{A93A59EC-D75D-44E1-9720-F75D9EF95BC3}.Release|x64.Build.0 = Release|Any CPU
 		{A93A59EC-D75D-44E1-9720-F75D9EF95BC3}.Release|x86.ActiveCfg = Release|Any CPU
 		{A93A59EC-D75D-44E1-9720-F75D9EF95BC3}.Release|x86.Build.0 = Release|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Debug|x64.Build.0 = Debug|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Debug|x86.Build.0 = Debug|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Release|Any CPU.Build.0 = Release|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Release|x64.ActiveCfg = Release|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Release|x64.Build.0 = Release|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Release|x86.ActiveCfg = Release|Any CPU
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -84,6 +100,7 @@ Global
 		{8239FC82-46A3-4F21-8D05-1F0BE0B1B1FC} = {712ED94A-F982-4667-A9CE-E8F21900BBED}
 		{E90F7470-C7F8-464B-9C28-87C474085812} = {712ED94A-F982-4667-A9CE-E8F21900BBED}
 		{A93A59EC-D75D-44E1-9720-F75D9EF95BC3} = {E3EF31E0-6658-4899-8BDA-FF84355E2FDD}
+		{2AA5AC6B-531B-426E-AD38-5B03F1949CF5} = {07C56E10-A807-4372-ACD9-ADED2D099BC8}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {4DB894E3-1BF2-4410-911A-14D32FD79A96}

diff --git a/src/EntityFramework.Storage/host/ConsoleHost/ConsoleHost.csproj b/src/EntityFramework.Storage/host/ConsoleHost/ConsoleHost.csproj
@@ -0,0 +1,17 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <TargetFramework>netcoreapp3.1</TargetFramework>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <FrameworkReference Include="Microsoft.AspNetCore.App" />
+    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\..\src\IdentityServer4.EntityFramework.Storage.csproj" />
+  </ItemGroup>
+
+</Project>
diff --git a/src/EntityFramework.Storage/host/ConsoleHost/Program.cs b/src/EntityFramework.Storage/host/ConsoleHost/Program.cs
@@ -0,0 +1,35 @@
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using System;
+using IdentityServer4.EntityFramework.Storage;
+using Microsoft.EntityFrameworkCore;
+using IdentityServer4.EntityFramework;
+
+namespace ConsoleHost
+{
+    class Program
+    {
+        static void Main(string[] args)
+        {
+            var connectionString = "server=(localdb)\\mssqllocaldb;database=IdentityServer4.EntityFramework-4.0.0;trusted_connection=yes;";
+
+            var services = new ServiceCollection();
+            services.AddLogging(b => b.AddConsole().SetMinimumLevel(LogLevel.Trace));
+            services.AddOperationalDbContext(options =>
+            {
+                options.ConfigureDbContext = builder => builder.UseSqlServer(connectionString);
+
+                // this enables automatic token cleanup. this is optional.
+                options.EnableTokenCleanup = false;
+                options.TokenCleanupInterval = 5; // interval in seconds, short for testing
+            });
+
+            var sp = services.BuildServiceProvider();
+            using (var scope = sp.CreateScope())
+            {
+                var svc = scope.ServiceProvider.GetRequiredService<TokenCleanupService>();
+                svc.RemoveExpiredGrantsAsync().GetAwaiter().GetResult();
+            }
+        }
+    }
+}
diff --git a/src/EntityFramework.Storage/src/TokenCleanup/TokenCleanupService.cs b/src/EntityFramework.Storage/src/TokenCleanup/TokenCleanupService.cs
@@ -5,6 +5,7 @@
 using System;
 using System.Linq;
 using System.Threading.Tasks;
+using IdentityServer4.EntityFramework.Entities;
 using IdentityServer4.EntityFramework.Interfaces;
 using IdentityServer4.EntityFramework.Options;
 using Microsoft.EntityFrameworkCore;
@@ -85,25 +86,17 @@ protected virtual async Task RemoveGrantsAsync()
                 if (found > 0)
                 {
                     _persistedGrantDbContext.PersistedGrants.RemoveRange(expiredGrants);
-                    try
-                    {
-                        await _persistedGrantDbContext.SaveChangesAsync();
+                    await SaveChangesAsync();
 
-                        if (_operationalStoreNotification != null)
-                        {
-                            await _operationalStoreNotification.PersistedGrantsRemovedAsync(expiredGrants);
-                        }
-                    }
-                    catch (DbUpdateConcurrencyException ex)
+                    if (_operationalStoreNotification != null)
                     {
-                        // we get this if/when someone else already deleted the records
-                        // we want to essentially ignore this, and keep working
-                        _logger.LogDebug("Concurrency exception removing expired grants: {exception}", ex.Message);
+                        await _operationalStoreNotification.PersistedGrantsRemovedAsync(expiredGrants);
                     }
                 }
             }
         }
 
+
         /// <summary>
         /// Removes the stale device codes.
         /// </summary>
@@ -126,23 +119,44 @@ protected virtual async Task RemoveDeviceCodesAsync()
                 if (found > 0)
                 {
                     _persistedGrantDbContext.DeviceFlowCodes.RemoveRange(expiredCodes);
-                    try
-                    {
-                        await _persistedGrantDbContext.SaveChangesAsync();
+                    await SaveChangesAsync();
 
-                        if (_operationalStoreNotification != null)
-                        {
-                            await _operationalStoreNotification.DeviceCodesRemovedAsync(expiredCodes);
-                        }
+                    if (_operationalStoreNotification != null)
+                    {
+                        await _operationalStoreNotification.DeviceCodesRemovedAsync(expiredCodes);
                     }
-                    catch (DbUpdateConcurrencyException ex)
+                }
+            }
+        }
+
+        private async Task SaveChangesAsync()
+        {
+            var count = 3;
+
+            while (count > 0)
+            {
+                try
+                {
+                    await _persistedGrantDbContext.SaveChangesAsync();
+                    return;
+                }
+                catch (DbUpdateConcurrencyException ex)
+                {
+                    count--;
+
+                    // we get this if/when someone else already deleted the records
+                    // we want to essentially ignore this, and keep working
+                    _logger.LogDebug("Concurrency exception removing expired grants: {exception}", ex.Message);
+
+                    foreach (var entry in ex.Entries)
                     {
-                        // we get this if/when someone else already deleted the records
-                        // we want to essentially ignore this, and keep working
-                        _logger.LogDebug("Concurrency exception removing expired grants: {exception}", ex.Message);
+                        // mark this entry as not attached anymore so we don't try to re-delete
+                        entry.State = EntityState.Detached;
                     }
                 }
             }
+
+            _logger.LogDebug("Too many concurrency exceptions. Exiting.");
         }
     }
 }