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

Await using #436

Merged
merged 36 commits into from
Aug 21, 2023
Merged

Await using #436

merged 36 commits into from
Aug 21, 2023

Conversation

sburmanoctopus
Copy link
Contributor

@sburmanoctopus sburmanoctopus commented Aug 17, 2023

[sc-53211]

Background

Continuing on from making disposing async, we need to make disposal with using statements become async.

Results

Related to OctopusDeploy/Issues#8266

Before

Whenever something that supports async operations is disposed, it does the disposal synchronously on the thread. This blocks the thread if disposal involves things like IO.

After

We now try to use await using instead of using do that any asynchronous operations done during disposal will no longer block the calling thread.

.NET 4.8 Complications

Streams in .NET 4.8 do not support IAsyncDisposable. This means we cannot add await to using statements.

This was fixed by wrapping the await in a #if

RewindableBufferStream

We also made RewindableBufferStream implement IAsyncDisposable. This allowed us to using await using whenever we interacted with this type of stream.

Using Our Streams

We made all our streams extend IAsyncDisposable so that they would work in .NET 4.8 with await using

How to review this PR

Quality ✔️

Pre-requisites

  • I have read How we use GitHub Issues for help deciding when and where it's appropriate to make an issue.
  • I have considered informing or consulting the right people, according to the ownership map.
  • I have considered appropriate testing for my change.

# Conflicts:
#	source/Halibut.Tests/Transport/ConnectionManagerFixture.cs
# Conflicts:
#	source/Halibut.Tests/Transport/ConnectionPoolFixture.cs
#	source/Halibut/Transport/ConnectionManager.cs
#	source/Halibut/Transport/ConnectionPool.cs
# Conflicts:
#	source/Halibut.Tests/Support/TestConnection.cs
#	source/Halibut.Tests/Transport/ConnectionPoolFixture.cs
#	source/Halibut/HalibutRuntime.cs
#	source/Halibut/Transport/ConnectionManager.cs
#	source/Halibut/Transport/ConnectionManagerAsync.cs
#	source/Halibut/Transport/ConnectionPool.cs
#	source/Halibut/Transport/ConnectionPoolAsync.cs
#	source/Halibut/Transport/IConnectionManager.cs
#	source/Halibut/Transport/IConnectionPool.cs
#	source/Halibut/Transport/SecureClient.cs
#	source/Halibut/Transport/SecureListeningClient.cs
#	source/Halibut/Transport/SecureWebSocketClient.cs
# Conflicts:
#	source/Halibut/ServiceModel/PendingRequestQueueAsync.cs
#	source/Halibut/Transport/SecureConnection.cs
# Conflicts:
#	source/Halibut/Transport/ConnectionManager.cs
#	source/Halibut/Transport/ConnectionPool.cs
@shortcut-integration
Copy link

@nathanwoctopusdeploy
Copy link
Contributor

[sc-57124]

# Conflicts:
#	source/Halibut.Tests/Timeouts/SendingAndReceivingRequestMessagesTimeoutsFixture.cs
#	source/Halibut/Transport/DiscoveryClient.cs
#	source/Halibut/Transport/Protocol/MessageSerializer.cs
#	source/Halibut/Transport/SecureListener.cs
#	source/Halibut/Transport/SecureWebSocketListener.cs
@shortcut-integration
Copy link

var e = (await AssertAsync.Throws<HalibutClientException>(() => echoServiceTheErrorWillHappenOn.SayHelloAsync(stringToSend))).And;
var e = (await AssertAsync.Throws<HalibutClientException>(() =>
{
var stringToSend = Some.RandomAsciiStringOfLength(numberOfBytesBeforePausingAStream * 20);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was driving me nuts debugging in this area, and having VS take ages to "evaluate local variables" because of this large string.

So I made sure it was no longer in scope. Solved!

@@ -23,7 +23,13 @@ public TcpTunnel(TcpClient fromClient, TcpClient toClient, ILogger logger)

public async Task Tunnel(CancellationToken cancellationToken)
{
#if !NETFRAMEWORK
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If only dotnet had #define or better yet virtual threads.

@sburmanoctopus sburmanoctopus marked this pull request as ready for review August 21, 2023 00:12
@sburmanoctopus sburmanoctopus requested a review from a team as a code owner August 21, 2023 00:12
@sburmanoctopus sburmanoctopus merged commit ac3442a into main Aug 21, 2023
10 of 14 checks passed
@sburmanoctopus sburmanoctopus deleted the sast/AwaitUsing branch August 21, 2023 00:14
@sburmanoctopus
Copy link
Contributor Author

[sc-57123]

@shortcut-integration
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants