cyberpresidentvanellope/tgstation-server
1
0
Fork 0
mirror of https://github.com/tgstation/tgstation-server.git synced 2026-08-23 13:07:07 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1552 from tgstation/FixThatRaceCondition [TGSDeploy]

Browse Source 
Prevent reboot bridge request completing too soon. v5.12.6 release
This commit is contained in:
Jordan Dominion
2023-06-18 23:54:51 -04:00
committed by GitHub
parent db488ea01d d169f4b653
commit b597ed1f96
7 changed files with 69 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Tgstation.Server.Host/Components/Session/ISessionController.cs
+5
View File
@@ -69,6 +69,11 @@ namespace Tgstation.Server.Host.Components.Session
/// </summary>
Task OnReboot { get; }
/// <summary>
/// A <see cref="Task"/> that must complete before a TgsReboot() bridge request can complete.
/// </summary>
Task RebootGate { set; }
/// <summary>
/// A <see cref="Task"/> that completes when the server calls /world/TgsInitializationComplete().
/// </summary>
src/Tgstation.Server.Host/Components/Session/SessionController.cs
+28
View File
@@ -75,6 +75,26 @@ namespace Tgstation.Server.Host.Components.Session
/// <inheritdoc />
public Task OnReboot => rebootTcs.Task;
/// <inheritdoc />
public Task RebootGate
{
get => rebootGate;
set
{
var tcs = new TaskCompletionSource();
Task toAwait = null;
async Task Wrap()
{
await tcs.Task;
await toAwait;
await value;
}
toAwait = Interlocked.Exchange(ref rebootGate, Wrap());
tcs.SetResult();
}
}
/// <inheritdoc />
public Task OnPrime => primeTcs.Task;
@@ -164,6 +184,11 @@ namespace Tgstation.Server.Host.Components.Session
/// </summary>
volatile TaskCompletionSource primeTcs;
/// <summary>
/// Backing field for <see cref="RebootGate"/>.
/// </summary>
volatile Task rebootGate;
/// <summary>
/// The number of currently active calls to <see cref="ProcessBridgeRequest(BridgeParameters, CancellationToken)"/> from TgsReboot().
/// </summary>
@@ -254,6 +279,8 @@ namespace Tgstation.Server.Host.Components.Session
rebootTcs = new TaskCompletionSource();
primeTcs = new TaskCompletionSource();
rebootGate = Task.CompletedTask;
// Run this asynchronously because we want to try to avoid any effects sending topics to the server while the initial bridge request is processing
// It MAY be the source of a DD crash. See this gist https://gist.github.com/Cyberboss/7776bbeff3a957d76affe0eae95c9f14
// Worth further investigation as to if that sequence of events is a reliable crash vector and opening a BYOND bug if it is
@@ -751,6 +778,7 @@ namespace Tgstation.Server.Host.Components.Session
}
Interlocked.Exchange(ref rebootTcs, new TaskCompletionSource()).SetResult();
await RebootGate;
}
finally
{
src/Tgstation.Server.Host/Components/Watchdog/WatchdogBase.cs
+8
View File
@@ -804,6 +804,8 @@ namespace Tgstation.Server.Host.Components.Watchdog
var ranInitialDmbCheck = false;
for (ulong iteration = 1; nextAction != MonitorAction.Exit; ++iteration)
using (LogContext.PushProperty(SerilogContextHelper.WatchdogMonitorIterationContextProperty, iteration))
{
TaskCompletionSource nextMonitorWakeupTcs = new TaskCompletionSource();
try
{
Logger.LogTrace("Iteration {iteration} of monitor loop", iteration);
@@ -821,6 +823,7 @@ namespace Tgstation.Server.Host.Components.Watchdog
oldTask = newTaskFactory();
}
controller.RebootGate = nextMonitorWakeupTcs.Task;
if (lastController == controller)
{
TryUpdateTask(ref activeServerLifetime, () => controller.Lifetime);
@@ -967,6 +970,11 @@ namespace Tgstation.Server.Host.Components.Watchdog
nextAction = MonitorAction.Continue;
}
}
finally
{
nextMonitorWakeupTcs.SetResult();
}
}
}
catch (OperationCanceledException)
{
src/Tgstation.Server.Host/Controllers/ComponentInterfacingController.cs
+5 -2
View File
@@ -115,14 +115,17 @@ namespace Tgstation.Server.Host.Controllers
/// Run a given <paramref name="action"/> with the relevant <see cref="IInstance"/>.
/// </summary>
/// <param name="action">A <see cref="Func{T, TResult}"/> accepting the <see cref="IInstance"/> and returning a <see cref="Task{TResult}"/> with the <see cref="IActionResult"/>.</param>
/// <param name="instance">The <see cref="Models.Instance"/> to grab. If <see langword="null"/>, <see cref="ApiController.Instance"/> will be used.</param>
/// <returns>A <see cref="Task{TResult}"/> resulting in the <see cref="IActionResult"/> that should be returned.</returns>
/// <remarks>The context of <paramref name="action"/> should be as small as possible so as to avoid race conditions. This function can return a <see cref="ConflictResult"/> if the requested instance was offline.</remarks>
protected async Task<IActionResult> WithComponentInstance(Func<IInstanceCore, Task<IActionResult>> action)
protected async Task<IActionResult> WithComponentInstance(Func<IInstanceCore, Task<IActionResult>> action, Models.Instance instance = null)
{
if (action == null)
throw new ArgumentNullException(nameof(action));
using var instanceReference = instanceManager.GetInstanceReference(Instance);
instance ??= Instance;
using var instanceReference = instanceManager.GetInstanceReference(instance);
using (LogContext.PushProperty(SerilogContextHelper.InstanceReferenceContextProperty, instanceReference.Uid))
{
if (instanceReference == null)
src/Tgstation.Server.Host/Controllers/InstanceController.cs
+14 -10
View File
@@ -429,11 +429,13 @@ namespace Tgstation.Server.Host.Controllers
if (renamed)
{
// ignoring retval because we don't care if it's offline
await WithComponentInstance(async componentInstance =>
{
await componentInstance.InstanceRenamed(originalModel.Name, cancellationToken);
return null;
});
await WithComponentInstance(
async componentInstance =>
{
await componentInstance.InstanceRenamed(originalModel.Name, cancellationToken);
return null;
},
originalModel);
}
var oldAutoStart = originalModel.DreamDaemonSettings.AutoStart;
@@ -491,11 +493,13 @@ namespace Tgstation.Server.Host.Controllers
if (model.AutoUpdateInterval.HasValue && oldAutoUpdateInterval != model.AutoUpdateInterval)
{
// ignoring retval because we don't care if it's offline
await WithComponentInstance(async componentInstance =>
{
await componentInstance.SetAutoUpdateInterval(model.AutoUpdateInterval.Value);
return null;
});
await WithComponentInstance(
async componentInstance =>
{
await componentInstance.SetAutoUpdateInterval(model.AutoUpdateInterval.Value);
return null;
},
originalModel);
}
await CheckAccessible(api, cancellationToken);
tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs
+2 -1
View File
@@ -306,7 +306,8 @@ namespace Tgstation.Server.Tests.Live.Instance
|| job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: E_HANDLE: The handle is invalid.")
|| job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: 3489660936: Unknown error (0xd0000008)")
|| job.ExceptionDetails.Contains("System.InvalidOperationException: No process is associated with this object.")
|| job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: 2147942424: The program issued a command but the command length is incorrect."))))
|| job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: 2147942424: The program issued a command but the command length is incorrect.")
|| job.ExceptionDetails.Contains("BetterWin32Errors.Win32Exception: 2147942699: Only part of a ReadProcessMemory or WriteProcessMemory request was completed."))))
break;
var restartJob = await instanceClient.DreamDaemon.Restart(cancellationToken);
tests/Tgstation.Server.Tests/Live/InstanceManagerTest.cs
+7
View File
@@ -163,6 +163,13 @@ namespace Tgstation.Server.Tests.Live
Assert.AreEqual(ConfigurationType.HostWrite, firstTest.ConfigurationType);
Assert.IsTrue(Directory.Exists(firstTest.Path));
// regression check
await instanceManagerClient.Update(new InstanceUpdateRequest
{
Id = firstTest.Id,
AutoUpdateInterval = 9999,
}, cancellationToken);
//can't move online instance
await ApiAssert.ThrowsException<ConflictException>(() => instanceManagerClient.Update(new InstanceUpdateRequest
{