Closed nathanwoctopusdeploy closed 1 year ago
[sc-56801]
This pull request has been linked to Shortcut Story #56801: Fix Pending Request Queue Async AsyncManualResetEvent leaks and CancellationTokenSourceLeaks.
The bug in the queue can be reproduced with:
[Test]
public async Task leakTheQueue()
{
var queue = new PendingRequestQueueBuilder().WithSyncOrAsync(SyncOrAsync.Async).WithEndpoint("poll://foo")
.WithPollingQueueWaitTimeout(TimeSpan.FromMilliseconds(1))
.Build();
var cts = new CancellationTokenSource();
var tasks = new List<Task>();
for (int i = 0; i < 2000; i++)
{
tasks.Add(TryDequeue(queue, cts.Token));
}
await Task.WhenAll(tasks.ToArray());
}
static async Task TryDequeue(IPendingRequestQueue queue, CancellationToken cancellationToken)
{
for (int i = 0; i < 6520000; i++)
{
await queue.DequeueAsync(cancellationToken);
}
}
Background
The
PendingRequestQueueAsync
had to significantly change the way locking worked from thePendingRequestQueue
when converted to be async.This PR fixes some memory leaks identified when testing async Halibut in server.
This PR also adds a change to delete a PendingRequest from the
queue
when the code that queued the pending request has stopped waiting (cancelled or timed out). This allows the request to be disposed and also removes a known issues where null may be returned for DequeuNextAsync while there are items waiting to be dequeued and removes an un-necessary poll loop from the Polling Service to get the next request.Related to https://github.com/OctopusDeploy/Issues/issues/8266
How to review this PR
Quality :heavy_check_mark:
Pre-requisites