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

don't dispose the blocking collection when cancelling #222

Merged
merged 5 commits into from
Feb 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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 src/Speckle.Sdk/Api/Operations/Operations.Receive.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ CancellationToken cancellationToken

try
{
var process = serializeProcessFactory.CreateDeserializeProcess(
using var process = serializeProcessFactory.CreateDeserializeProcess(
url,
streamId,
authorizationToken,
Expand Down
26 changes: 17 additions & 9 deletions src/Speckle.Sdk/Serialisation/V2/Send/PriorityScheduler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ CancellationToken cancellationToken
) : TaskScheduler, IDisposable
{
#pragma warning disable CA2213
//intentionally not disposing this because syncing to when all the threads are done AFTER the BC is done/disposed is hard. BC will still be cleaned up by the finalizer
private readonly BlockingCollection<Task> _tasks = new();
#pragma warning restore CA2213
private Thread[]? _threads;
Expand All @@ -37,23 +38,30 @@ protected override void QueueTask(Task task)
{
try
{
foreach (Task t in _tasks.GetConsumingEnumerable(cancellationToken))
while (true)
{
if (cancellationToken.IsCancellationRequested)
//we're done so leave
if (_tasks.IsCompleted || cancellationToken.IsCancellationRequested)
{
break;
}

TryExecuteTask(t);
var success = _tasks.TryTake(out var t, TimeSpan.FromSeconds(1));
//no task and we're done so leave
if (success && _tasks.IsCompleted)
{
break;
}
//cancelled just leave
if (cancellationToken.IsCancellationRequested)
{
break;
}
}

if (_tasks.IsCompleted)
{
_tasks.Dispose();
//didn't get a task but just timed out so continue
if (!success)
{
continue;
}
TryExecuteTask(t ?? throw new InvalidOperationException("Task was null"));
}
}
catch (OperationCanceledException)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,33 @@

namespace Speckle.Sdk.Tests.Integration.API.GraphQL.Resources;

public class CommentResourceTests
public class CommentResourceTests : IAsyncLifetime
{
private readonly Client _testUser;
private readonly CommentResource Sut;
private readonly Project _project;
private readonly Model _model;
private readonly Version _version;
private readonly Comment _comment;
private Client _testUser;
private CommentResource Sut;
private Project _project;
private Model _model;
private Version _version;
private Comment _comment;

// Constructor for setup
public CommentResourceTests()
public async Task InitializeAsync()
{
// Synchronous operations converted to async Task.Run for constructor
_testUser = Task.Run(async () => await Fixtures.SeedUserWithClient()).Result;
_project = Task.Run(async () => await _testUser.Project.Create(new("Test project", "", null))).Result;
_model = Task.Run(async () => await _testUser.Model.Create(new("Test Model 1", "", _project.id))).Result;
_version = Task.Run(async () => await Fixtures.CreateVersion(_testUser, _project.id, _model.id)).Result;
_comment = Task.Run(CreateComment).Result!;
_testUser = await Fixtures.SeedUserWithClient();
_project = await _testUser.Project.Create(new("Test project", "", null));
_model = await _testUser.Model.Create(new("Test Model 1", "", _project.id));
_version = await Fixtures.CreateVersion(_testUser, _project.id, _model.id);
_comment = await CreateComment();
Sut = _testUser.Comment;
}

public Task DisposeAsync()
{
// No resources to dispose
return Task.CompletedTask;
}

[Fact]
public async Task Get()
{
Expand Down