Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v6.11.3: Add a bunch of logging to config writing and semaphore usage #1994

Merged
merged 2 commits into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion build/Version.props
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<!-- Integration tests will ensure they match across the board -->
<Import Project="WebpanelVersion.props" />
<PropertyGroup>
<TgsCoreVersion>6.11.2</TgsCoreVersion>
<TgsCoreVersion>6.11.3</TgsCoreVersion>
<TgsConfigVersion>5.3.0</TgsConfigVersion>
<TgsRestVersion>10.10.0</TgsRestVersion>
<TgsGraphQLVersion>0.2.0</TgsGraphQLVersion>
Expand Down
23 changes: 13 additions & 10 deletions src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public Configuration(
this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration));
this.sessionConfiguration = sessionConfiguration ?? throw new ArgumentNullException(nameof(sessionConfiguration));

semaphore = new SemaphoreSlim(1);
semaphore = new SemaphoreSlim(1, 1);
disposeCts = new CancellationTokenSource();
uploadTasks = Task.CompletedTask;
}
Expand All @@ -201,7 +201,7 @@ public void Dispose()
/// <inheritdoc />
public async ValueTask<ServerSideModifications?> CopyDMFilesTo(string dmeFile, string destination, CancellationToken cancellationToken)
{
using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken))
using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken, logger))
{
var ensureDirectoriesTask = EnsureDirectories(cancellationToken);

Expand Down Expand Up @@ -270,7 +270,7 @@ void ListImpl()
}));
}

using (SemaphoreSlimContext.TryLock(semaphore, out var locked))
using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked))
{
if (!locked)
{
Expand Down Expand Up @@ -376,7 +376,7 @@ void GetFileStream()
}
}

using (SemaphoreSlimContext.TryLock(semaphore, out var locked))
using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked))
{
if (!locked)
{
Expand Down Expand Up @@ -434,7 +434,7 @@ await ValueTaskExtensions.WhenAll(entries.Select<string, ValueTask>(async file =
}));
}

using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken))
using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken, logger))
{
await EnsureDirectories(cancellationToken);
var ignoreFileBytes = await ioManager.ReadAllBytes(StaticIgnorePath(), cancellationToken);
Expand Down Expand Up @@ -501,14 +501,15 @@ void WriteCallback()
return;
}

using (SemaphoreSlimContext.TryLock(semaphore, out var locked))
using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked))
{
if (!locked)
{
fileTicket.SetError(ErrorCode.ConfigurationContendedAccess, null);
return;
}

logger.LogTrace("Kicking off write callback");
if (systemIdentity == null)
await Task.Factory.StartNew(WriteCallback, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current);
else
Expand All @@ -519,6 +520,8 @@ void WriteCallback()
fileTicket.SetError(ErrorCode.ConfigurationFileUpdated, fileHash);
else if (uploadStream.Length > 0)
postWriteHandler.HandleWrite(path);
else
logger.LogTrace("Write complete");
}
}

Expand Down Expand Up @@ -559,7 +562,7 @@ void WriteCallback()
}
}

using (SemaphoreSlimContext.TryLock(semaphore, out var locked))
using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked))
{
if (!locked)
{
Expand All @@ -585,7 +588,7 @@ void WriteCallback()
bool? result = null;
void DoCreate() => result = synchronousIOManager.CreateDirectory(path, cancellationToken);

using (SemaphoreSlimContext.TryLock(semaphore, out var locked))
using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked))
{
if (!locked)
{
Expand Down Expand Up @@ -651,7 +654,7 @@ public ValueTask HandleEvent(EventType eventType, IEnumerable<string?> parameter
var path = ValidateConfigRelativePath(configurationRelativePath);

var result = false;
using (SemaphoreSlimContext.TryLock(semaphore, out var locked))
using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked))
{
if (!locked)
{
Expand Down Expand Up @@ -750,7 +753,7 @@ async ValueTask ExecuteEventScripts(IEnumerable<string?> parameters, bool deploy
await EnsureDirectories(cancellationToken);

// always execute in serial
using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken))
using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken, logger))
{
var files = await ioManager.GetFilesWithExtension(EventScriptsSubdirectory, platformIdentifier.ScriptFileExtension, false, cancellationToken);
var resolvedScriptsDir = ioManager.ResolvePath(EventScriptsSubdirectory);
Expand Down
30 changes: 24 additions & 6 deletions src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
using System.Threading;
using System.Threading.Tasks;

using Microsoft.Extensions.Logging;

namespace Tgstation.Server.Host.Utils
{
/// <summary>
Expand All @@ -14,29 +16,39 @@ sealed class SemaphoreSlimContext : IDisposable
/// </summary>
/// <param name="semaphore">The <see cref="SemaphoreSlim"/> to lock.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <param name="logger">An optional <see cref="ILogger"/> to write to.</param>
/// <returns>A <see cref="Task{TResult}"/> resulting in the <see cref="SemaphoreSlimContext"/> for the lock.</returns>
public static async ValueTask<SemaphoreSlimContext> Lock(SemaphoreSlim semaphore, CancellationToken cancellationToken)
public static async ValueTask<SemaphoreSlimContext> Lock(SemaphoreSlim semaphore, CancellationToken cancellationToken, ILogger? logger = null)
{
ArgumentNullException.ThrowIfNull(semaphore);
logger?.LogTrace("Acquiring semaphore...");
await semaphore.WaitAsync(cancellationToken);
return new SemaphoreSlimContext(semaphore);
return new SemaphoreSlimContext(semaphore, logger);
}

/// <summary>
/// Asyncronously attempts to lock a <paramref name="semaphore"/>.
/// </summary>
/// <param name="semaphore">The <see cref="SemaphoreSlim"/> to lock.</param>
/// <param name="logger">An optional <see cref="ILogger"/> to write to.</param>
/// <param name="locked">The <see cref="bool"/> result of the lock attempt.</param>
/// <returns>A <see cref="SemaphoreSlimContext"/> for the lock on success, or <see langword="null"/> if it was not acquired.</returns>
public static SemaphoreSlimContext? TryLock(SemaphoreSlim semaphore, out bool locked)
public static SemaphoreSlimContext? TryLock(SemaphoreSlim semaphore, ILogger? logger, out bool locked)
{
ArgumentNullException.ThrowIfNull(semaphore);
logger?.LogTrace("Trying to acquire semaphore...");
locked = semaphore.Wait(TimeSpan.Zero);
logger?.LogTrace("Acquired semaphore {un}successfully", locked ? String.Empty : "un");
return locked
? new SemaphoreSlimContext(semaphore)
? new SemaphoreSlimContext(semaphore, logger)
: null;
}

/// <summary>
/// An optional <see cref="ILogger"/> to write to.
/// </summary>
readonly ILogger? logger;

/// <summary>
/// The locked <see cref="SemaphoreSlim"/>.
/// </summary>
Expand All @@ -46,14 +58,20 @@ public static async ValueTask<SemaphoreSlimContext> Lock(SemaphoreSlim semaphore
/// Initializes a new instance of the <see cref="SemaphoreSlimContext"/> class.
/// </summary>
/// <param name="lockedSemaphore">The value of <see cref="lockedSemaphore"/>.</param>
SemaphoreSlimContext(SemaphoreSlim lockedSemaphore)
/// <param name="logger">The value of <see cref="logger"/>.</param>
SemaphoreSlimContext(SemaphoreSlim lockedSemaphore, ILogger? logger)
{
this.lockedSemaphore = lockedSemaphore;
this.logger = logger;
}

/// <summary>
/// Release the lock on <see cref="lockedSemaphore"/>.
/// </summary>
public void Dispose() => lockedSemaphore.Release();
public void Dispose()
{
logger?.LogTrace("Releasing semaphore...");
lockedSemaphore.Release();
}
}
}
Loading