From 34a7e58bd4466eabf80918d067570f5ee68d6836 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 3 Jun 2023 00:39:19 -0400 Subject: [PATCH 1/3] Work around for potential deadlock in DiscordProvider Dispose Workaround for https://github.com/Remora/Remora.Discord/issues/305 --- .../Chat/Providers/DiscordProvider.cs | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index be488af4bb..c06ac872c6 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -3,7 +3,9 @@ using System.Collections.Generic; using System.Drawing; using System.Globalization; using System.Linq; +using System.Reflection; using System.Threading; +using System.Threading.Channels; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; @@ -17,6 +19,7 @@ using Remora.Discord.API.Abstractions.Results; using Remora.Discord.API.Objects; using Remora.Discord.Gateway; using Remora.Discord.Gateway.Extensions; +using Remora.Discord.Gateway.Services; using Remora.Rest.Core; using Remora.Rest.Results; using Remora.Results; @@ -223,7 +226,35 @@ namespace Tgstation.Server.Host.Components.Chat.Providers } await base.DisposeAsync(); - await serviceProvider.DisposeAsync(); + +#if NET7_0_OR_GREATER +#error This hack needs to be removed after updating Remora.Discord +#endif + + // https://github.com/Remora/Remora.Discord/issues/305 + var responderDispatchService = serviceProvider.GetRequiredService(); + var serviceProviderDisposeTask = serviceProvider.DisposeAsync().AsTask(); + var timeout = AsyncDelayer.Delay(TimeSpan.FromSeconds(5), default); // DCT: None available + + await Task.WhenAny(timeout, serviceProviderDisposeTask); + + if (!serviceProviderDisposeTask.IsCompleted) + { + // HACK HACK HACK, there's a potential deadlock in the ResponderDispatchService + var responderDispatchServiceType = responderDispatchService.GetType(); + var dispatcherTask = (Task)responderDispatchServiceType.GetField("_dispatcher", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(responderDispatchService); + var finalizerTask = (Task)responderDispatchServiceType.GetField("_finalizer", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(responderDispatchService); + + if (dispatcherTask.IsCompleted && !finalizerTask.IsCompleted) + { + // deadlocked, force close the channel + var channel = (Channel>>)responderDispatchServiceType.GetField("_respondersToFinalize", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(responderDispatchService); + channel.Writer.TryComplete(); + } + } + + await serviceProviderDisposeTask; + Logger.LogTrace("ServiceProvider disposed"); // this line is purely here to shutup CA2213. It should always be null @@ -660,6 +691,8 @@ namespace Tgstation.Server.Host.Components.Chat.Providers localGatewayCts.Cancel(); var gatewayResult = await localGatewayTask; + + Logger.LogTrace("Gateway task complete"); if (!gatewayResult.IsSuccess) Logger.LogWarning("Gateway issue: {result}", gatewayResult.LogFormat()); From 7fd744f9dc7cebd2ef4ad061e7d9746342904e01 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 3 Jun 2023 01:37:05 -0400 Subject: [PATCH 2/3] Longer workaround delay, additional logging --- .../Chat/Providers/DiscordProvider.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index c06ac872c6..478a93a4b2 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -234,23 +234,34 @@ namespace Tgstation.Server.Host.Components.Chat.Providers // https://github.com/Remora/Remora.Discord/issues/305 var responderDispatchService = serviceProvider.GetRequiredService(); var serviceProviderDisposeTask = serviceProvider.DisposeAsync().AsTask(); - var timeout = AsyncDelayer.Delay(TimeSpan.FromSeconds(5), default); // DCT: None available + var timeout = AsyncDelayer.Delay(TimeSpan.FromSeconds(10), default); // DCT: None available await Task.WhenAny(timeout, serviceProviderDisposeTask); if (!serviceProviderDisposeTask.IsCompleted) { // HACK HACK HACK, there's a potential deadlock in the ResponderDispatchService + Logger.LogWarning("ServiceProvider disposal stalled. Attempting workaround..."); var responderDispatchServiceType = responderDispatchService.GetType(); var dispatcherTask = (Task)responderDispatchServiceType.GetField("_dispatcher", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(responderDispatchService); var finalizerTask = (Task)responderDispatchServiceType.GetField("_finalizer", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(responderDispatchService); - if (dispatcherTask.IsCompleted && !finalizerTask.IsCompleted) + var dispatcherCompleted = dispatcherTask.IsCompleted; + var finalizerCompleted = finalizerTask.IsCompleted; + if (dispatcherCompleted && !finalizerCompleted) { // deadlocked, force close the channel var channel = (Channel>>)responderDispatchServiceType.GetField("_respondersToFinalize", BindingFlags.Instance | BindingFlags.NonPublic).GetValue(responderDispatchService); - channel.Writer.TryComplete(); + if (!channel.Writer.TryComplete()) + Logger.LogCritical("Workaround failed (channel already closed), you may be deadlocked!"); + else + Logger.LogInformation("Workaround seems successful. Awaiting ServiceProvider disposal..."); } + else + Logger.LogCritical( + "Workaround failed (_dispatcher: {dispatcherCompleted}, _finalizer: {finalizerCompleted}), you may be deadlocked!", + dispatcherCompleted, + finalizerCompleted); } await serviceProviderDisposeTask; From ac2c7aebf5b3cd091e2b64081ffdb65304d053d8 Mon Sep 17 00:00:00 2001 From: Dominion Date: Sat, 3 Jun 2023 02:34:13 -0400 Subject: [PATCH 3/3] Drain the channel too --- .../Components/Chat/Providers/DiscordProvider.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs index 478a93a4b2..1fbdf6ed53 100644 --- a/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs +++ b/src/Tgstation.Server.Host/Components/Chat/Providers/DiscordProvider.cs @@ -209,7 +209,7 @@ namespace Tgstation.Server.Host.Components.Chat.Providers serviceProvider = new ServiceCollection() .AddDiscordGateway(serviceProvider => botToken) .Configure(options => options.Intents |= GatewayIntents.MessageContents) - .AddSingleton(serviceProvider => this) + .AddSingleton(serviceProvider => (IDiscordResponders)this) .AddResponder() .BuildServiceProvider(); } @@ -255,7 +255,21 @@ namespace Tgstation.Server.Host.Components.Chat.Providers if (!channel.Writer.TryComplete()) Logger.LogCritical("Workaround failed (channel already closed), you may be deadlocked!"); else + { + // drain the channel, fuck the results + // DCT: None available + await foreach (var result in channel.Reader.ReadAllAsync(default)) + try + { + await result; + } + catch (Exception ex) + { + Logger.LogDebug(ex, "Channel draining exception!"); + } + Logger.LogInformation("Workaround seems successful. Awaiting ServiceProvider disposal..."); + } } else Logger.LogCritical(