Skip to content

Commit

Permalink
Fix game directory leaks in HardLinkDmbProvider
Browse files Browse the repository at this point in the history
- Adjust tests to account for expected directories correctly.
- Better logging of leftover directories.
- Fix reboot not being waited on in a test case.
  • Loading branch information
Cyberboss committed Jul 15, 2024
1 parent d370c2c commit 8401f3c
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 35 deletions.
106 changes: 78 additions & 28 deletions src/Tgstation.Server.Host/Components/Deployment/HardLinkDmbProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ sealed class HardLinkDmbProvider : SwappableDmbProvider
/// <summary>
/// The <see cref="Task"/> representing the base provider mirroring operation.
/// </summary>
readonly Task<string> mirroringTask;
readonly Task<string?> mirroringTask;

/// <summary>
/// The <see cref="ILogger"/> for the <see cref="HardLinkDmbProvider"/>.
Expand Down Expand Up @@ -77,13 +77,27 @@ public override async ValueTask DisposeAsync()
{
cancellationTokenSource.Cancel();
cancellationTokenSource.Dispose();
try
var mirroredDir = await mirroringTask;
if (mirroredDir != null && !Swapped)
{
await mirroringTask;
}
catch (OperationCanceledException ex)
{
logger.LogDebug(ex, "Mirroring task cancelled!");
logger.LogDebug("Cancelled mirroring task, we must cleanup!");

// We shouldn't be doing long running I/O ops because this could be under an HTTP request (DELETE /api/DreamDaemon)
// dirty shit to follow:
async void AsyncCleanup()
{
try
{
await IOManager.DeleteDirectory(mirroredDir, CancellationToken.None); // DCT: None available
logger.LogTrace("Completed async cleanup of unused mirror directory: {mirroredDir}", mirroredDir);
}
catch (Exception ex)
{
logger.LogError(ex, "Error cleaning up mirrored directory {mirroredDir}!", mirroredDir);
}
}

AsyncCleanup();
}

await base.DisposeAsync();
Expand All @@ -103,6 +117,13 @@ protected override async ValueTask DoSwap(CancellationToken cancellationToken)
{
logger.LogTrace("Begin DoSwap, mirroring task complete: {complete}...", mirroringTask.IsCompleted);
var mirroredDir = await mirroringTask.WaitAsync(cancellationToken);
if (mirroredDir == null)
{
// huh, how?
cancellationToken.ThrowIfCancellationRequested();
throw new InvalidOperationException("mirroringTask was cancelled without us being cancelled?");
}

var goAheadTcs = new TaskCompletionSource();

// I feel dirty...
Expand All @@ -119,6 +140,7 @@ async void DisposeOfOldDirectory()
goAheadTcs.SetResult();
logger.LogTrace("Deleting old Live directory {path}...", disposePath);
await IOManager.DeleteDirectory(disposePath, CancellationToken.None); // DCT: We're detached at this point
logger.LogTrace("Completed async cleanup of old Live directory: {disposePath}", disposePath);
}
catch (DirectoryNotFoundException ex)
{
Expand Down Expand Up @@ -148,31 +170,59 @@ async void DisposeOfOldDirectory()
/// <param name="securityLevel">The launch <see cref="DreamDaemonSecurity"/> level.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <returns>A <see cref="Task{TResult}"/> resulting in the full path to the mirrored directory.</returns>
async Task<string> MirrorSourceDirectory(int? taskThrottle, DreamDaemonSecurity securityLevel, CancellationToken cancellationToken)
async Task<string?> MirrorSourceDirectory(int? taskThrottle, DreamDaemonSecurity securityLevel, CancellationToken cancellationToken)
{
var stopwatch = Stopwatch.StartNew();
var mirrorGuid = Guid.NewGuid();
logger.LogDebug("Starting to mirror {sourceDir} as hard links to {mirrorGuid}...", CompileJob.DirectoryName, mirrorGuid);
if (taskThrottle.HasValue && taskThrottle < 1)
throw new ArgumentOutOfRangeException(nameof(taskThrottle), taskThrottle, "taskThrottle must be at least 1!");

var src = IOManager.ResolvePath(CompileJob.DirectoryName!.Value.ToString());
var dest = IOManager.ResolvePath(mirrorGuid.ToString());

using var semaphore = taskThrottle.HasValue ? new SemaphoreSlim(taskThrottle.Value) : null;
await Task.WhenAll(MirrorDirectoryImpl(
src,
dest,
semaphore,
securityLevel,
cancellationToken));
stopwatch.Stop();

logger.LogDebug(
"Finished mirror of {sourceDir} to {mirrorGuid} in {seconds}s...",
CompileJob.DirectoryName,
mirrorGuid,
stopwatch.Elapsed.TotalSeconds.ToString("0.##", CultureInfo.InvariantCulture));
string? dest = null;
try
{
var stopwatch = Stopwatch.StartNew();
var mirrorGuid = Guid.NewGuid();

logger.LogDebug("Starting to mirror {sourceDir} as hard links to {mirrorGuid}...", CompileJob.DirectoryName, mirrorGuid);

var src = IOManager.ResolvePath(CompileJob.DirectoryName!.Value.ToString());
dest = IOManager.ResolvePath(mirrorGuid.ToString());

using var semaphore = taskThrottle.HasValue ? new SemaphoreSlim(taskThrottle.Value) : null;
await Task.WhenAll(MirrorDirectoryImpl(
src,
dest,
semaphore,
securityLevel,
cancellationToken));
stopwatch.Stop();

logger.LogDebug(
"Finished mirror of {sourceDir} to {mirrorGuid} in {seconds}s...",
CompileJob.DirectoryName,
mirrorGuid,
stopwatch.Elapsed.TotalSeconds.ToString("0.##", CultureInfo.InvariantCulture));
}
catch (OperationCanceledException ex)
{
logger.LogDebug(ex, "Cancelled while mirroring");
}
catch (Exception ex)
{
logger.LogError(ex, "Could not mirror!");

if (dest != null)
try
{
logger.LogDebug("Cleaning up mirror attempt: {dest}", dest);
await IOManager.DeleteDirectory(dest, cancellationToken);
}
catch (OperationCanceledException ex2)
{
logger.LogDebug(ex2, "Errored cleanup cancellation edge case!");
return dest;
}

return null;
}

return dest;
}
Expand Down
11 changes: 9 additions & 2 deletions tests/Tgstation.Server.Tests/Live/Instance/InstanceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,12 @@ public async Task RunTests(
ddPort,
usingBasicWatchdog);
await wdt.Run(cancellationToken);
await wdt.ExpectGameDirectoryCount(2, cancellationToken);

await wdt.ExpectGameDirectoryCount(
usingBasicWatchdog || new PlatformIdentifier().IsWindows
? 2 // old + new deployment
: 3, // + new mirrored deployment waiting to take over Live
cancellationToken);
}

public static async ValueTask<IEngineInstallationData> DownloadEngineVersion(
Expand Down Expand Up @@ -285,7 +290,9 @@ await instanceClient.Repository.Update(new RepositoryUpdateRequest

await instanceClient.DreamDaemon.Shutdown(cancellationToken);

await wdt.ExpectGameDirectoryCount(1, cancellationToken);
await wdt.ExpectGameDirectoryCount(
1, // current deployment
cancellationToken);

await instanceManagerClient.Update(new InstanceUpdateRequest
{
Expand Down
9 changes: 5 additions & 4 deletions tests/Tgstation.Server.Tests/Live/Instance/WatchdogTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -633,15 +633,16 @@ async Task TestDMApiFreeDeploy(CancellationToken cancellationToken)

public async ValueTask ExpectGameDirectoryCount(int expected, CancellationToken cancellationToken)
{
string[] lastDirectories;
int CountNonLiveDirs()
{
var directories = Directory.GetDirectories(Path.Combine(instanceClient.Metadata.Path, "Game"));
return directories.Where(directory => Path.GetFileName(directory) != "Live").Count();
lastDirectories = Directory.GetDirectories(Path.Combine(instanceClient.Metadata.Path, "Game"));
return lastDirectories.Where(directory => Path.GetFileName(directory) != "Live").Count();
}

int nonLiveDirs = 0;
// cleanup task is async
for(var i = 0; i < 15; ++i)
for(var i = 0; i < 20; ++i)
{
nonLiveDirs = CountNonLiveDirs();
if (expected == nonLiveDirs)
Expand All @@ -651,7 +652,7 @@ int CountNonLiveDirs()
}

nonLiveDirs = CountNonLiveDirs();
Assert.AreEqual(expected, nonLiveDirs);
Assert.AreEqual(expected, nonLiveDirs, $"Directories present: {String.Join(", ", lastDirectories.Select(Path.GetFileName))}");
}

async Task RunBasicTest(CancellationToken cancellationToken)
Expand Down
2 changes: 1 addition & 1 deletion tests/Tgstation.Server.Tests/Live/TestLiveServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1724,7 +1724,7 @@ async Task WaitForInitialJobs(IInstanceClient instanceClient)
Assert.AreEqual(dd.StagedCompileJob.Job.Id, compileJob.Id);

expectedCompileJobId = compileJob.Id.Value;
dd = await wdt.TellWorldToReboot(server.UsingBasicWatchdog, cancellationToken);
dd = await wdt.TellWorldToReboot(true, cancellationToken);

Assert.AreEqual(dd.ActiveCompileJob.Job.Id, expectedCompileJobId);
Assert.AreEqual(WatchdogStatus.Online, dd.Status.Value);
Expand Down

0 comments on commit 8401f3c

Please sign in to comment.