diff --git a/Remora.Discord.Extensions/Remora.Discord.Extensions.csproj b/Remora.Discord.Extensions/Remora.Discord.Extensions.csproj index 00b6684a88..8f4298dbdc 100644 --- a/Remora.Discord.Extensions/Remora.Discord.Extensions.csproj +++ b/Remora.Discord.Extensions/Remora.Discord.Extensions.csproj @@ -1,7 +1,7 @@  - 5.3.5 + 5.3.6 Utilities and components which extend upon Remora.Discord's base resources Update dependencies. diff --git a/Remora.Discord.Interactivity/Remora.Discord.Interactivity.csproj b/Remora.Discord.Interactivity/Remora.Discord.Interactivity.csproj index 8ff122629c..50bbcc03a8 100644 --- a/Remora.Discord.Interactivity/Remora.Discord.Interactivity.csproj +++ b/Remora.Discord.Interactivity/Remora.Discord.Interactivity.csproj @@ -1,10 +1,10 @@ - 4.5.4 + 5.0.0 Framework for using Discord's interaction-driven message components - Update dependencies. + BREAKING: Rework deletion logic for data leases to prevent deadlocks. diff --git a/Remora.Discord.Interactivity/Services/DataLease.cs b/Remora.Discord.Interactivity/Services/DataLease.cs index e495a91ede..b6c6eee9c6 100644 --- a/Remora.Discord.Interactivity/Services/DataLease.cs +++ b/Remora.Discord.Interactivity/Services/DataLease.cs @@ -37,13 +37,17 @@ namespace Remora.Discord.Interactivity.Services; public class DataLease : IAsyncDisposable where TKey : notnull { private readonly InMemoryDataService _dataService; - private readonly TKey _key; private readonly SemaphoreSlim _semaphore; private bool _shouldDelete; private bool _isDisposed; private TData _data; + /// + /// Gets the key associated with the lease. + /// + public TKey Key { get; } + /// /// Gets or sets the data associated with the lease. /// @@ -73,9 +77,10 @@ public TData Data internal DataLease(InMemoryDataService dataService, TKey key, SemaphoreSlim semaphore, TData data) { _dataService = dataService; - _key = key; _semaphore = semaphore; _data = data; + + this.Key = key; } /// @@ -94,34 +99,22 @@ public async ValueTask DisposeAsync() return; } - _isDisposed = true; - GC.SuppressFinalize(this); - if (_shouldDelete) + try { - var couldDelete = await _dataService.DeleteDataAsync(_key); - if (couldDelete) + if (_shouldDelete) { + _ = await _dataService.TryDeleteDataAsync(this); return; } - // manual cleanup, someone must have deleted the data while we were using it - if (_data is IAsyncDisposable asyncDisposableData) - { - await asyncDisposableData.DisposeAsync(); - } - - if (_data is IDisposable disposableData) - { - disposableData.Dispose(); - } - - _semaphore.Dispose(); - return; + _dataService.UpdateData(this.Key, _data); + } + finally + { + _isDisposed = true; + _semaphore.Release(); } - - _dataService.UpdateData(_key, _data); - _semaphore.Release(); } } diff --git a/Remora.Discord.Interactivity/Services/InMemoryDataService.cs b/Remora.Discord.Interactivity/Services/InMemoryDataService.cs index ada52f76bd..074a784fc5 100644 --- a/Remora.Discord.Interactivity/Services/InMemoryDataService.cs +++ b/Remora.Discord.Interactivity/Services/InMemoryDataService.cs @@ -100,126 +100,92 @@ public async Task>> LeaseDataAsync(TKey key, Cance var (semaphore, data) = tuple; await semaphore.WaitAsync(ct); - return new DataLease(this, key, semaphore, data); - } - - /// - /// Rents the data associated with the given key, blocking until a lock can be taken on the data object. - /// - /// - /// The semaphore returned by this method has the lock held on it and must be released once the caller is done with - /// the object. - /// - /// The key the data object is associated with. - /// The cancellation token for this operation. - /// The data and semaphore associated with the data or a . - [Obsolete("Use LeaseDataAsync instead.", true)] - public async Task> RentDataAsync - ( - TKey key, - CancellationToken ct = default - ) - { - if (_isDisposed) - { - throw new ObjectDisposedException("The data service has been disposed of and cannot be used."); - } - - if (!_data.TryGetValue(key, out var tuple)) + // may have been deleted while we were waiting - check first + if (_data.ContainsKey(key)) { - return new NotFoundError(); + return new DataLease(this, key, semaphore, data); } - var (semaphore, _) = tuple; - await semaphore.WaitAsync(ct); - - return tuple; + semaphore.Release(); + return new NotFoundError(); } /// - /// Removes the data associated with the given key. This method does nothing if no data is associated with - /// the given key. + /// Deletes the data associated with the given lease. /// - /// The key the data object is associated with. + /// The lease you have on the data object. /// true if the data was successfully removed; otherwise, false. - public bool TryRemoveData(TKey key) + public bool TryDeleteData(DataLease lease) { if (_isDisposed) { throw new ObjectDisposedException("The data service has been disposed of and cannot be used."); } - if (!_data.TryRemove(key, out var tuple)) + if (!_data.TryRemove(lease.Key, out _)) { + // already removed return false; } - var (semaphore, data) = tuple; - - if (data is IAsyncDisposable and not IDisposable) + if (lease.Data is IAsyncDisposable and not IDisposable) { throw new InvalidOperationException ( - $"Unable to synchronously dispose of the held data belonging to key {key}." + $"Unable to synchronously dispose of the held data belonging to key {lease.Key}." ); } - if (data is IDisposable disposableData) - { - disposableData.Dispose(); - } - - if (data is IAsyncDisposable and not IDisposable) + if (lease.Data is IDisposable disposable) { - throw new InvalidOperationException - ( - $"Unable to synchronously dispose of the held data belonging to key {key}." - ); + disposable.Dispose(); } - semaphore.Dispose(); return true; } /// - /// Removes the data associated with the given key. This method does nothing if no data is associated with - /// the given key. + /// Deletes the data associated with the given lease. /// - /// The key the data object is associated with. + /// The lease you have on the data object. /// true if the data was successfully removed; otherwise, false. - public async ValueTask TryRemoveDataAsync(TKey key) + public async Task TryDeleteDataAsync(DataLease lease) { if (_isDisposed) { throw new ObjectDisposedException("The data service has been disposed of and cannot be used."); } - if (!_data.TryRemove(key, out var tuple)) + if (!_data.TryRemove(lease.Key, out _)) { + // already removed return false; } - var (semaphore, data) = tuple; - - if (data is IAsyncDisposable asyncDisposableData) - { - await asyncDisposableData.DisposeAsync(); - } - - if (data is IDisposable disposableData) + switch (lease.Data) { - disposableData.Dispose(); + // preferentially use the asynchronous disposal logic + case IAsyncDisposable asyncDisposable: + { + await asyncDisposable.DisposeAsync(); + break; + } + case IDisposable disposable: + { + disposable.Dispose(); + break; + } } - semaphore.Dispose(); return true; } /// - /// Deletes the data, disposing of the semaphore and, if necessary, the data. + /// Deletes the data associated with the key, disposing of the data if necessary. A lock is acquired before the data + /// is disposed. /// /// The key the data object is associated with. - /// true if the data was successfully removed; otherwise, false.. + /// true if the data was successfully removed; otherwise, false. internal async ValueTask DeleteDataAsync(TKey key) { if (_isDisposed) @@ -229,22 +195,26 @@ internal async ValueTask DeleteDataAsync(TKey key) if (!_data.TryRemove(key, out var tuple)) { - // already gone return false; } var (semaphore, data) = tuple; - if (data is IAsyncDisposable asyncDisposableData) - { - await asyncDisposableData.DisposeAsync(); - } + await semaphore.WaitAsync(); - if (data is IDisposable disposableData) + switch (data) { - disposableData.Dispose(); + case IAsyncDisposable asyncDisposableData: + { + await asyncDisposableData.DisposeAsync(); + break; + } + case IDisposable disposableData: + { + disposableData.Dispose(); + break; + } } - semaphore.Dispose(); return true; } diff --git a/Remora.Discord.Pagination/Remora.Discord.Pagination.csproj b/Remora.Discord.Pagination/Remora.Discord.Pagination.csproj index 11a26b372a..e7dfceba68 100644 --- a/Remora.Discord.Pagination/Remora.Discord.Pagination.csproj +++ b/Remora.Discord.Pagination/Remora.Discord.Pagination.csproj @@ -1,13 +1,10 @@ - 4.0.0 + 4.0.1 Button-based pagination of messages for Remora.Discord Update dependencies. - BREAKING: Change help text in pagination to a full embed. - Make help text embeds ephemeral by default. - Fix that the initial message in the pagination doesn't use the footer format. diff --git a/Remora.Discord.Pagination/Responders/MessageDeletedResponder.cs b/Remora.Discord.Pagination/Responders/MessageDeletedResponder.cs index b95b7b2b05..ad14716708 100644 --- a/Remora.Discord.Pagination/Responders/MessageDeletedResponder.cs +++ b/Remora.Discord.Pagination/Responders/MessageDeletedResponder.cs @@ -49,7 +49,15 @@ public MessageDeletedResponder(InMemoryDataService public async Task RespondAsync(IMessageDelete gatewayEvent, CancellationToken ct = default) { - _ = await _paginationData.TryRemoveDataAsync(gatewayEvent.ID); + var getLease = await _paginationData.LeaseDataAsync(gatewayEvent.ID, ct); + if (!getLease.IsSuccess) + { + return Result.FromSuccess(); + } + + await using var lease = getLease.Entity; + lease.Delete(); + return Result.FromSuccess(); } @@ -58,7 +66,14 @@ public async Task RespondAsync(IMessageDeleteBulk gatewayEvent, Cancella { foreach (var id in gatewayEvent.IDs) { - _ = await _paginationData.TryRemoveDataAsync(id); + var getLease = await _paginationData.LeaseDataAsync(id, ct); + if (!getLease.IsSuccess) + { + continue; + } + + await using var lease = getLease.Entity; + lease.Delete(); } return Result.FromSuccess(); diff --git a/Remora.Discord/Remora.Discord.csproj b/Remora.Discord/Remora.Discord.csproj index c4bb7e9236..fb34aba735 100644 --- a/Remora.Discord/Remora.Discord.csproj +++ b/Remora.Discord/Remora.Discord.csproj @@ -1,21 +1,11 @@ - 2024.2 + 2024.3 Metapackage for Remora.Discord's various components Update dependencies. - BREAKING: Change help text in pagination to a full embed. - BREAKING: Implement bulk banning endpoint. - BREAKING: Implement support for nonce enforcement. - BREAKING: Implement support for one-time purchases. - BREAKING: Implement support for polls. - BREAKING: Implement support for user applications. - Fix that the initial message in the pagination doesn't use the footer format. - Handle reconnection requests on connect. - Implement support for parsing partial messages. - Make help text embeds ephemeral by default. - Update maximum command length. + BREAKING: Rework deletion logic for data leases to prevent deadlocks.