diff --git a/src/Tgstation.Server.Api/Models/ErrorCode.cs b/src/Tgstation.Server.Api/Models/ErrorCode.cs index a2e59a6856..23f8be21af 100644 --- a/src/Tgstation.Server.Api/Models/ErrorCode.cs +++ b/src/Tgstation.Server.Api/Models/ErrorCode.cs @@ -288,9 +288,8 @@ namespace Tgstation.Server.Api.Models /// /// Currently unused. /// - [Obsolete("Unused", true)] - [Description("Unknown error code.")] - UnusedErrorCode3, + [Description("IO operation could not start contended access to the instance's configuration directory!")] + ConfigurationContendedAccess, /// /// Attempted to add a chat bot when at or above the or it was set to something lower than the existing amount of chat bots. diff --git a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs index 350d701880..1e7f082fe8 100644 --- a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs +++ b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs @@ -237,35 +237,34 @@ namespace Tgstation.Server.Host.Components.StaticFiles void ListImpl() { - try + var enumerator = synchronousIOManager.GetDirectories(path, cancellationToken); + result.AddRange(enumerator.Select(x => new ConfigurationFileResponse { - var enumerator = synchronousIOManager.GetDirectories(path, cancellationToken); - result.AddRange(enumerator.Select(x => new ConfigurationFileResponse - { - IsDirectory = true, - Path = ioManager.ConcatPath(configurationRelativePath, x), - }).OrderBy(file => file.Path)); + IsDirectory = true, + Path = ioManager.ConcatPath(configurationRelativePath, x), + }).OrderBy(file => file.Path)); - enumerator = synchronousIOManager.GetFiles(path, cancellationToken); - result.AddRange(enumerator.Select(x => new ConfigurationFileResponse - { - IsDirectory = false, - Path = ioManager.ConcatPath(configurationRelativePath, x), - }).OrderBy(file => file.Path)); - } - catch (IOException ex) + enumerator = synchronousIOManager.GetFiles(path, cancellationToken); + result.AddRange(enumerator.Select(x => new ConfigurationFileResponse { - logger.LogDebug(ex, "IOException while enumerating direcotry!"); - result = null; - return; - } + IsDirectory = false, + Path = ioManager.ConcatPath(configurationRelativePath, x), + }).OrderBy(file => file.Path)); } - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + { + if (!locked) + { + logger.LogDebug("Contention when attempting to enumerate directory!"); + return null; + } + if (systemIdentity == null) ListImpl(); else await systemIdentity.RunImpersonated(ListImpl, cancellationToken); + } return result; } @@ -280,90 +279,102 @@ namespace Tgstation.Server.Host.Components.StaticFiles void ReadImpl() { - lock (semaphore) + try + { + string GetFileSha() + { + var content = synchronousIOManager.ReadFile(path); + using var sha1 = SHA1.Create(); + return String.Join(String.Empty, sha1.ComputeHash(content).Select(b => b.ToString("x2", CultureInfo.InvariantCulture))); + } + + var originalSha = GetFileSha(); + + var disposeToken = disposeCts.Token; + var fileTicket = fileTransferService.CreateDownload( + new FileDownloadProvider( + () => + { + if (disposeToken.IsCancellationRequested) + return ErrorCode.InstanceOffline; + + var newSha = GetFileSha(); + if (newSha != originalSha) + return ErrorCode.ConfigurationFileUpdated; + + return null; + }, + async cancellationToken => + { + FileStream result = null; + void GetFileStream() + { + result = ioManager.GetFileStream(path, false); + } + + using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + { + if (!locked) + return null; + + if (systemIdentity == null) + await Task.Factory.StartNew(GetFileStream, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); + else + await systemIdentity.RunImpersonated(GetFileStream, cancellationToken); + } + + return result; + }, + path, + false)); + + result = new ConfigurationFileResponse + { + FileTicket = fileTicket.FileTicket, + IsDirectory = false, + LastReadHash = originalSha, + AccessDenied = false, + Path = configurationRelativePath, + }; + } + catch (UnauthorizedAccessException) + { + // this happens on windows, dunno about linux + bool isDirectory; try { - string GetFileSha() - { - var content = synchronousIOManager.ReadFile(path); - using var sha1 = SHA1.Create(); - return String.Join(String.Empty, sha1.ComputeHash(content).Select(b => b.ToString("x2", CultureInfo.InvariantCulture))); - } - - var originalSha = GetFileSha(); - - var disposeToken = disposeCts.Token; - var fileTicket = fileTransferService.CreateDownload( - new FileDownloadProvider( - () => - { - if (disposeToken.IsCancellationRequested) - return ErrorCode.InstanceOffline; - - var newSha = GetFileSha(); - if (newSha != originalSha) - return ErrorCode.ConfigurationFileUpdated; - - return null; - }, - async cancellationToken => - { - FileStream result = null; - void GetFileStream() - { - result = ioManager.GetFileStream(path, false); - } - - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) - if (systemIdentity == null) - await Task.Factory.StartNew(GetFileStream, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); - else - await systemIdentity.RunImpersonated(GetFileStream, cancellationToken); - - return result; - }, - path, - false)); - - result = new ConfigurationFileResponse - { - FileTicket = fileTicket.FileTicket, - IsDirectory = false, - LastReadHash = originalSha, - AccessDenied = false, - Path = configurationRelativePath, - }; + isDirectory = synchronousIOManager.IsDirectory(path); } - catch (UnauthorizedAccessException) + catch (Exception ex) { - // this happens on windows, dunno about linux - bool isDirectory; - try - { - isDirectory = synchronousIOManager.IsDirectory(path); - } - catch (Exception ex) - { - logger.LogDebug(ex, "IsDirectory exception!"); - isDirectory = false; - } - - result = new ConfigurationFileResponse - { - Path = configurationRelativePath, - }; - if (!isDirectory) - result.AccessDenied = true; - - result.IsDirectory = isDirectory; + logger.LogDebug(ex, "IsDirectory exception!"); + isDirectory = false; } + + result = new ConfigurationFileResponse + { + Path = configurationRelativePath, + }; + if (!isDirectory) + result.AccessDenied = true; + + result.IsDirectory = isDirectory; + } } - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + { + if (!locked) + { + logger.LogDebug("Contention when attempting to read file!"); + return null; + } + if (systemIdentity == null) await Task.Factory.StartNew(ReadImpl, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); else await systemIdentity.RunImpersonated(ReadImpl, cancellationToken); + } return result; } @@ -448,93 +459,108 @@ namespace Tgstation.Server.Host.Components.StaticFiles void WriteImpl() { - lock (semaphore) - try + try + { + var fileTicket = fileTransferService.CreateUpload(FileUploadStreamKind.ForSynchronousIO); + var uploadCancellationToken = disposeCts.Token; + async Task UploadHandler() { - var fileTicket = fileTransferService.CreateUpload(FileUploadStreamKind.ForSynchronousIO); - var uploadCancellationToken = disposeCts.Token; - async Task UploadHandler() + await using (fileTicket) { - await using (fileTicket) + var fileHash = previousHash; + var uploadStream = await fileTicket.GetResult(uploadCancellationToken); + if (uploadStream == null) + return; // expired + + bool success = false; + void WriteCallback() { - var fileHash = previousHash; - var uploadStream = await fileTicket.GetResult(uploadCancellationToken); - if (uploadStream == null) - return; // expired + success = synchronousIOManager.WriteFileChecked(path, uploadStream, ref fileHash, cancellationToken); + } - bool success = false; - void WriteCallback() - { - success = synchronousIOManager.WriteFileChecked(path, uploadStream, ref fileHash, cancellationToken); - } + if (fileTicket == null) + { + logger.LogDebug("File upload ticket for {path} expired!", path); + return; + } - if (fileTicket == null) + using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + { + if (!locked) { - logger.LogDebug("File upload ticket for {path} expired!", path); + fileTicket.SetError(ErrorCode.ConfigurationContendedAccess, null); return; } - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) - if (systemIdentity == null) - await Task.Factory.StartNew(WriteCallback, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); - else - await systemIdentity.RunImpersonated(WriteCallback, cancellationToken); - - if (!success) - fileTicket.SetError(ErrorCode.ConfigurationFileUpdated, fileHash); - else if (uploadStream.Length > 0) - postWriteHandler.HandleWrite(path); + if (systemIdentity == null) + await Task.Factory.StartNew(WriteCallback, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); + else + await systemIdentity.RunImpersonated(WriteCallback, cancellationToken); } + + if (!success) + fileTicket.SetError(ErrorCode.ConfigurationFileUpdated, fileHash); + else if (uploadStream.Length > 0) + postWriteHandler.HandleWrite(path); } - - result = new ConfigurationFileResponse - { - FileTicket = fileTicket.Ticket.FileTicket, - LastReadHash = previousHash, - IsDirectory = false, - AccessDenied = false, - Path = configurationRelativePath, - }; - - lock (disposeCts) - uploadTasks = Task.WhenAll(uploadTasks, UploadHandler()); } - catch (UnauthorizedAccessException) + + result = new ConfigurationFileResponse { - // this happens on windows, dunno about linux - bool isDirectory; - try - { - isDirectory = synchronousIOManager.IsDirectory(path); - } - catch (Exception ex) - { - logger.LogDebug(ex, "IsDirectory exception!"); - isDirectory = false; - } + FileTicket = fileTicket.Ticket.FileTicket, + LastReadHash = previousHash, + IsDirectory = false, + AccessDenied = false, + Path = configurationRelativePath, + }; - result = new ConfigurationFileResponse - { - Path = configurationRelativePath, - }; - if (!isDirectory) - result.AccessDenied = true; - - result.IsDirectory = isDirectory; + lock (disposeCts) + uploadTasks = Task.WhenAll(uploadTasks, UploadHandler()); + } + catch (UnauthorizedAccessException) + { + // this happens on windows, dunno about linux + bool isDirectory; + try + { + isDirectory = synchronousIOManager.IsDirectory(path); } + catch (Exception ex) + { + logger.LogDebug(ex, "IsDirectory exception!"); + isDirectory = false; + } + + result = new ConfigurationFileResponse + { + Path = configurationRelativePath, + }; + if (!isDirectory) + result.AccessDenied = true; + + result.IsDirectory = isDirectory; + } } - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + { + if (!locked) + { + logger.LogDebug("Contention when attempting to write file!"); + return null; + } + if (systemIdentity == null) await Task.Factory.StartNew(WriteImpl, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); else await systemIdentity.RunImpersonated(WriteImpl, cancellationToken); + } return result; } /// - public async Task CreateDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken) + public async Task CreateDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken) { await EnsureDirectories(cancellationToken); var path = ValidateConfigRelativePath(configurationRelativePath); @@ -542,11 +568,19 @@ namespace Tgstation.Server.Host.Components.StaticFiles bool? result = null; void DoCreate() => result = synchronousIOManager.CreateDirectory(path, cancellationToken); - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + { + if (!locked) + { + logger.LogDebug("Contention when attempting to create directory!"); + return null; + } + if (systemIdentity == null) await Task.Factory.StartNew(DoCreate, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); else await systemIdentity.RunImpersonated(DoCreate, cancellationToken); + } return result.Value; } @@ -618,14 +652,20 @@ namespace Tgstation.Server.Host.Components.StaticFiles } /// - public async Task DeleteDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken) + public async Task DeleteDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken) { await EnsureDirectories(cancellationToken); var path = ValidateConfigRelativePath(configurationRelativePath); var result = false; - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) { + if (!locked) + { + logger.LogDebug("Contention when attempting to enumerate directory!"); + return null; + } + void CheckDeleteImpl() => result = synchronousIOManager.DeleteDirectory(path); if (systemIdentity != null) diff --git a/src/Tgstation.Server.Host/Components/StaticFiles/IConfiguration.cs b/src/Tgstation.Server.Host/Components/StaticFiles/IConfiguration.cs index bc8b2be171..4fece676c9 100644 --- a/src/Tgstation.Server.Host/Components/StaticFiles/IConfiguration.cs +++ b/src/Tgstation.Server.Host/Components/StaticFiles/IConfiguration.cs @@ -38,7 +38,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles /// The relative path in the Configuration directory. /// The for the operation. If , the operation will be performed as the user of the . /// The for the operation. - /// A resulting in the s for the items in the directory. and will both be . + /// A resulting in the s for the items in the directory. and will both be . will be returned if the operation failed due to access contention. Task> ListDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken); /// @@ -47,7 +47,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles /// The relative path in the Configuration directory. /// The for the operation. If , the operation will be performed as the user of the . /// The for the operation. - /// A resulting in the of the file. + /// A resulting in the of the file. will be returned if the operation failed due to access contention. Task Read(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken); /// @@ -56,8 +56,8 @@ namespace Tgstation.Server.Host.Components.StaticFiles /// The relative path in the Configuration directory. /// The for the operation. If , the operation will be performed as the user of the . /// The for the operation. Usage may result in partial writes. - /// A resulting in if the directory already existed, otherwise. - Task CreateDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken); + /// A resulting in if the directory already existed, otherwise. will be returned if the operation failed due to access contention. + Task CreateDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken); /// /// Attempt to delete an empty directory at . @@ -65,8 +65,8 @@ namespace Tgstation.Server.Host.Components.StaticFiles /// The path of the empty directory to delete. /// The for the operation. If , the operation will be performed as the user of the . /// The for the operation. - /// if the directory was empty and deleted, otherwise. - Task DeleteDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken); + /// if the directory was empty and deleted, otherwise. will be returned if the operation failed due to access contention. + Task DeleteDirectory(string configurationRelativePath, ISystemIdentity systemIdentity, CancellationToken cancellationToken); /// /// Writes to a given . @@ -75,7 +75,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles /// The for the operation. If , the operation will be performed as the user of the . /// The hash any existing file must match in order for the write to succeed. /// The for the operation. Usage may result in partial writes. - /// A resulting in the updated or if the write failed due to conflicts. + /// A resulting in the updated and associated writing . will be returned if the operation failed due to access contention. Task Write(string configurationRelativePath, ISystemIdentity systemIdentity, string previousHash, CancellationToken cancellationToken); } } diff --git a/src/Tgstation.Server.Host/Controllers/ConfigurationController.cs b/src/Tgstation.Server.Host/Controllers/ConfigurationController.cs index 373554c928..52819df9ab 100644 --- a/src/Tgstation.Server.Host/Controllers/ConfigurationController.cs +++ b/src/Tgstation.Server.Host/Controllers/ConfigurationController.cs @@ -14,7 +14,6 @@ using Tgstation.Server.Api.Models.Response; using Tgstation.Server.Api.Rights; using Tgstation.Server.Host.Components; using Tgstation.Server.Host.Database; -using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.Security; @@ -86,6 +85,9 @@ namespace Tgstation.Server.Host.Controllers model.LastReadHash, cancellationToken); + if (newFile == null) + return Conflict(new ErrorMessageResponse(ErrorCode.ConfigurationContendedAccess)); + return model.LastReadHash == null ? Accepted(newFile) : Json(newFile); }); } @@ -114,6 +116,7 @@ namespace Tgstation.Server.Host.Controllers [HttpGet(Routes.File + "/{*filePath}")] [TgsAuthorize(ConfigurationRights.Read)] [ProducesResponseType(typeof(ConfigurationFileResponse), 200)] + [ProducesResponseType(typeof(ErrorMessageResponse), 409)] [ProducesResponseType(typeof(ErrorMessageResponse), 410)] public async Task File(string filePath, CancellationToken cancellationToken) { @@ -128,8 +131,9 @@ namespace Tgstation.Server.Host.Controllers var result = await instance .Configuration .Read(filePath, systemIdentity, cancellationToken); + if (result == null) - return this.Gone(); + return Conflict(new ErrorMessageResponse(ErrorCode.ConfigurationContendedAccess)); return Json(result); }); @@ -161,6 +165,7 @@ namespace Tgstation.Server.Host.Controllers [HttpGet(Routes.List + "/{*directoryPath}")] [TgsAuthorize(ConfigurationRights.List)] [ProducesResponseType(typeof(PaginatedResponse), 200)] + [ProducesResponseType(typeof(ErrorMessageResponse), 409)] [ProducesResponseType(typeof(ErrorMessageResponse), 410)] public Task Directory( string directoryPath, @@ -180,8 +185,10 @@ namespace Tgstation.Server.Host.Controllers var result = await instance .Configuration .ListDirectory(directoryPath, systemIdentity, cancellationToken); + if (result == null) - return new PaginatableResult(this.Gone()); + return new PaginatableResult( + Conflict(new ErrorMessageResponse(ErrorCode.ConfigurationContendedAccess))); return new PaginatableResult( result @@ -198,6 +205,15 @@ namespace Tgstation.Server.Host.Controllers return new PaginatableResult( Forbid()); } + catch (IOException ex) + { + Logger.LogInformation(ex, "IOException while enumerating directory!"); + return new PaginatableResult( + Conflict(new ErrorMessageResponse(ErrorCode.IOError) + { + AdditionalData = ex.Message, + })); + } }, null, page, @@ -231,6 +247,7 @@ namespace Tgstation.Server.Host.Controllers [TgsAuthorize(ConfigurationRights.Write)] [ProducesResponseType(typeof(ConfigurationFileResponse), 200)] [ProducesResponseType(typeof(ConfigurationFileResponse), 201)] + [ProducesResponseType(typeof(ErrorMessageResponse), 409)] public async Task CreateDirectory([FromBody] ConfigurationFileRequest model, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(model); @@ -247,11 +264,19 @@ namespace Tgstation.Server.Host.Controllers }; return await WithComponentInstance( - async instance => await instance - .Configuration - .CreateDirectory(model.Path, systemIdentity, cancellationToken) + async instance => + { + var result = await instance + .Configuration + .CreateDirectory(model.Path, systemIdentity, cancellationToken); + + if (!result.HasValue) + return Conflict(new ErrorMessageResponse(ErrorCode.ConfigurationContendedAccess)); + + return result.Value ? Json(resultModel) - : Created(resultModel)); + : Created(resultModel); + }); } catch (IOException e) { @@ -281,6 +306,7 @@ namespace Tgstation.Server.Host.Controllers [HttpDelete] [TgsAuthorize(ConfigurationRights.Delete)] [ProducesResponseType(204)] + [ProducesResponseType(typeof(ErrorMessageResponse), 409)] public async Task DeleteDirectory([FromBody] ConfigurationFileRequest directory, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(directory); @@ -294,11 +320,19 @@ namespace Tgstation.Server.Host.Controllers try { return await WithComponentInstance( - async instance => await instance - .Configuration - .DeleteDirectory(directory.Path, systemIdentity, cancellationToken) - ? NoContent() - : Conflict(new ErrorMessageResponse(ErrorCode.ConfigurationDirectoryNotEmpty))); + async instance => + { + var result = await instance + .Configuration + .DeleteDirectory(directory.Path, systemIdentity, cancellationToken); + + if (!result.HasValue) + return Conflict(new ErrorMessageResponse(ErrorCode.ConfigurationContendedAccess)); + + return result.Value + ? NoContent() + : Conflict(new ErrorMessageResponse(ErrorCode.ConfigurationDirectoryNotEmpty)); + }); } catch (NotImplementedException ex) { @@ -308,6 +342,14 @@ namespace Tgstation.Server.Host.Controllers { return Forbid(); } + catch (IOException ex) + { + Logger.LogInformation(ex, "IOException while deleting directory!"); + return Conflict(new ErrorMessageResponse(ErrorCode.IOError) + { + Message = ex.Message, + }); + } } /// diff --git a/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs b/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs index cdbe250071..25f070cc4b 100644 --- a/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs +++ b/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs @@ -7,7 +7,7 @@ namespace Tgstation.Server.Host.Utils /// /// Async lock context helper. /// - public sealed class SemaphoreSlimContext : IDisposable + sealed class SemaphoreSlimContext : IDisposable { /// /// Asyncronously locks a . @@ -15,14 +15,28 @@ namespace Tgstation.Server.Host.Utils /// The to lock. /// The for the operation. /// A resulting in the for the lock. - public static async Task Lock(SemaphoreSlim semaphore, CancellationToken cancellationToken) + public static async ValueTask Lock(SemaphoreSlim semaphore, CancellationToken cancellationToken) { ArgumentNullException.ThrowIfNull(semaphore); - cancellationToken.ThrowIfCancellationRequested(); await semaphore.WaitAsync(cancellationToken); return new SemaphoreSlimContext(semaphore); } + /// + /// Asyncronously attempts to lock a . + /// + /// The to lock. + /// The result of the lock attempt. + /// A for the lock on success, or if it was not acquired. + public static SemaphoreSlimContext TryLock(SemaphoreSlim semaphore, out bool locked) + { + ArgumentNullException.ThrowIfNull(semaphore); + locked = semaphore.Wait(TimeSpan.Zero); + return locked + ? new SemaphoreSlimContext(semaphore) + : null; + } + /// /// The locked . ///