-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-5777: Avoid ThreadPool-dependent IO methods in sync API #1805
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,44 +36,15 @@ public static void EfficientCopyTo(this Stream input, Stream output) | |
| } | ||
| } | ||
|
|
||
| public static int Read(this Stream stream, byte[] buffer, int offset, int count, TimeSpan timeout, CancellationToken cancellationToken) | ||
| { | ||
| try | ||
| { | ||
| using var manualResetEvent = new ManualResetEventSlim(); | ||
| var readOperation = stream.BeginRead( | ||
| buffer, | ||
| offset, | ||
| count, | ||
| state => ((ManualResetEventSlim)state.AsyncState).Set(), | ||
| manualResetEvent); | ||
|
|
||
| if (readOperation.IsCompleted || manualResetEvent.Wait(timeout, cancellationToken)) | ||
| { | ||
| return stream.EndRead(readOperation); | ||
| } | ||
| } | ||
| catch (OperationCanceledException) | ||
| { | ||
| // Have to suppress OperationCanceledException here, it will be thrown after the stream will be disposed. | ||
| } | ||
| catch (ObjectDisposedException) | ||
| { | ||
| throw new IOException(); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| stream.Dispose(); | ||
| } | ||
| catch | ||
| { | ||
| // Ignore any exceptions | ||
| } | ||
|
|
||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| throw new TimeoutException(); | ||
| } | ||
| public static int Read(this Stream stream, byte[] buffer, int offset, int count, TimeSpan timeout, CancellationToken cancellationToken) => | ||
| ExecuteOperationWithTimeout( | ||
| stream, | ||
| (str, state) => str.Read(state.Buffer, state.Offset, state.Count), | ||
| buffer, | ||
| offset, | ||
| count, | ||
| timeout, | ||
| cancellationToken); | ||
|
|
||
| public static async Task<int> ReadAsync(this Stream stream, byte[] buffer, int offset, int count, TimeSpan timeout, CancellationToken cancellationToken) | ||
| { | ||
|
|
@@ -217,46 +188,19 @@ public static async Task ReadBytesAsync(this Stream stream, byte[] destination, | |
| } | ||
| } | ||
|
|
||
| public static void Write(this Stream stream, byte[] buffer, int offset, int count, TimeSpan timeout, CancellationToken cancellationToken) | ||
| { | ||
| try | ||
| { | ||
| using var manualResetEvent = new ManualResetEventSlim(); | ||
| var writeOperation = stream.BeginWrite( | ||
| buffer, | ||
| offset, | ||
| count, | ||
| state => ((ManualResetEventSlim)state.AsyncState).Set(), | ||
| manualResetEvent); | ||
|
|
||
| if (writeOperation.IsCompleted || manualResetEvent.Wait(timeout, cancellationToken)) | ||
| public static void Write(this Stream stream, byte[] buffer, int offset, int count, TimeSpan timeout, CancellationToken cancellationToken) => | ||
| ExecuteOperationWithTimeout( | ||
| stream, | ||
| (str, state) => | ||
| { | ||
| stream.EndWrite(writeOperation); | ||
| return; | ||
| } | ||
| } | ||
| catch (OperationCanceledException) | ||
| { | ||
| // Have to suppress OperationCanceledException here, it will be thrown after the stream will be disposed. | ||
| } | ||
| catch (ObjectDisposedException) | ||
| { | ||
| // It's possible to get ObjectDisposedException when the connection pool was closed with interruptInUseConnections set to true. | ||
| throw new IOException(); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| stream.Dispose(); | ||
| } | ||
| catch | ||
| { | ||
| // Ignore any exceptions | ||
| } | ||
|
|
||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| throw new TimeoutException(); | ||
| } | ||
| str.Write(state.Buffer, state.Offset, state.Count); | ||
| return true; | ||
| }, | ||
| buffer, | ||
| offset, | ||
| count, | ||
| timeout, | ||
| cancellationToken); | ||
|
|
||
| public static async Task WriteAsync(this Stream stream, byte[] buffer, int offset, int count, TimeSpan timeout, CancellationToken cancellationToken) | ||
| { | ||
|
|
@@ -325,5 +269,86 @@ public static async Task WriteBytesAsync(this Stream stream, OperationContext op | |
| count -= bytesToWrite; | ||
| } | ||
| } | ||
|
|
||
| private static TResult ExecuteOperationWithTimeout<TResult>(Stream stream, Func<Stream, (byte[] Buffer, int Offset, int Count), TResult> operation, byte[] buffer, int offset, int count, TimeSpan timeout, CancellationToken cancellationToken) | ||
| { | ||
| StreamDisposeCallbackState callbackState = null; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we have tests to test all the major cases here? Timeout, cancellation, success.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tests added. |
||
| Timer timer = null; | ||
| CancellationTokenRegistration cancellationSubscription = default; | ||
| if (timeout != Timeout.InfiniteTimeSpan) | ||
| { | ||
| callbackState = new StreamDisposeCallbackState(stream); | ||
| timer = new Timer(DisposeStreamCallback, callbackState, timeout, Timeout.InfiniteTimeSpan); | ||
| } | ||
|
|
||
| if (cancellationToken.CanBeCanceled) | ||
| { | ||
| callbackState ??= new StreamDisposeCallbackState(stream); | ||
| cancellationSubscription = cancellationToken.Register(DisposeStreamCallback, callbackState); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| var result = operation(stream, (buffer, offset, count)); | ||
| if (callbackState?.TryChangeStateFromInProgress(OperationState.Done) == false) | ||
| { | ||
| // if cannot change the state - then the stream was/will be disposed, throw here | ||
| throw new IOException(); | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
| catch (IOException) | ||
| { | ||
| if (callbackState?.OperationState == OperationState.Interrupted) | ||
| { | ||
BorisDog marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| throw new TimeoutException(); | ||
| } | ||
|
|
||
| throw; | ||
| } | ||
| finally | ||
| { | ||
| timer?.Dispose(); | ||
| cancellationSubscription.Dispose(); | ||
| } | ||
|
|
||
| static void DisposeStreamCallback(object state) | ||
| { | ||
| var disposeCallbackState = (StreamDisposeCallbackState)state; | ||
| if (!disposeCallbackState.TryChangeStateFromInProgress(OperationState.Interrupted)) | ||
| { | ||
| // If the state can't be changed - then I/O had already succeeded | ||
| return; | ||
| } | ||
|
|
||
| try | ||
| { | ||
| disposeCallbackState.Stream.Dispose(); | ||
| } | ||
| catch (Exception) | ||
| { | ||
| // callbacks should not fail, suppress any exceptions here | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private record StreamDisposeCallbackState(Stream Stream) | ||
| { | ||
| private int _operationState = 0; | ||
|
|
||
| public OperationState OperationState => (OperationState)_operationState; | ||
|
|
||
| public bool TryChangeStateFromInProgress(OperationState newState) => | ||
| Interlocked.CompareExchange(ref _operationState, (int)newState, (int)OperationState.InProgress) == (int)OperationState.InProgress; | ||
| } | ||
|
|
||
| private enum OperationState | ||
| { | ||
| InProgress = 0, | ||
| Done, | ||
| Interrupted, | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| /* Copyright 2013-present MongoDB Inc. | ||
| /* Copyright 2010-present MongoDB Inc. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
|
|
@@ -90,20 +90,18 @@ public async Task ReadBytes_with_byte_array_should_have_expected_effect_for_part | |
| var bytes = new byte[] { 1, 2, 3 }; | ||
| var n = 0; | ||
| var position = 0; | ||
| Task<int> ReadPartial (byte[] buffer, int offset, int count) | ||
| int ReadPartial (byte[] buffer, int offset, int count) | ||
| { | ||
| var length = partition[n++]; | ||
| Buffer.BlockCopy(bytes, position, buffer, offset, length); | ||
| position += length; | ||
| return Task.FromResult(length); | ||
| return length; | ||
| } | ||
|
|
||
| mockStream.Setup(s => s.ReadAsync(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .Returns((byte[] buffer, int offset, int count, CancellationToken cancellationToken) => ReadPartial(buffer, offset, count)); | ||
| mockStream.Setup(s => s.BeginRead(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<AsyncCallback>(), It.IsAny<object>())) | ||
| .Returns((byte[] buffer, int offset, int count, AsyncCallback callback, object state) => ReadPartial(buffer, offset, count)); | ||
| mockStream.Setup(s => s.EndRead(It.IsAny<IAsyncResult>())) | ||
| .Returns<IAsyncResult>(x => ((Task<int>)x).GetAwaiter().GetResult()); | ||
| .Returns((byte[] buffer, int offset, int count, CancellationToken cancellationToken) => Task.FromResult(ReadPartial(buffer, offset, count))); | ||
| mockStream.Setup(s => s.Read(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>())) | ||
| .Returns((byte[] buffer, int offset, int count) => ReadPartial(buffer, offset, count)); | ||
| var destination = new byte[3]; | ||
|
|
||
| if (async) | ||
|
|
@@ -203,6 +201,49 @@ await Record.ExceptionAsync(() => stream.ReadBytesAsync(OperationContext.NoTimeo | |
| .ParamName.Should().Be("stream"); | ||
| } | ||
|
|
||
| [Theory] | ||
| [ParameterAttributeData] | ||
| public async Task ReadBytes_with_byte_array_throws_on_timeout([Values(true, false)]bool async) | ||
| { | ||
| var streamMock = new Mock<Stream>(); | ||
| SetupStreamRead(streamMock); | ||
| var stream = streamMock.Object; | ||
|
|
||
| var destination = new byte[2]; | ||
| var timeout = TimeSpan.FromMilliseconds(10); | ||
|
|
||
| var exception = async ? | ||
| await Record.ExceptionAsync(() => stream.ReadAsync(destination, 0, 2, timeout, CancellationToken.None)) : | ||
| Record.Exception(() => stream.Read(destination, 0, 2, timeout, CancellationToken.None)); | ||
|
|
||
| exception.Should().BeOfType<TimeoutException>(); | ||
| } | ||
|
|
||
| [Theory] | ||
| [ParameterAttributeData] | ||
| public async Task ReadBytes_with_byte_array_throws_on_cancellation([Values(true, false)]bool async) | ||
| { | ||
| var streamMock = new Mock<Stream>(); | ||
| SetupStreamRead(streamMock); | ||
| var stream = streamMock.Object; | ||
|
|
||
| var destination = new byte[2]; | ||
| using var cancellationTokenSource = new CancellationTokenSource(10); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice. |
||
|
|
||
| var exception = async ? | ||
| await Record.ExceptionAsync(() => stream.ReadAsync(destination, 0, 2, Timeout.InfiniteTimeSpan, cancellationTokenSource.Token)) : | ||
| Record.Exception(() => stream.Read(destination, 0, 2, Timeout.InfiniteTimeSpan, cancellationTokenSource.Token)); | ||
|
|
||
| if (async) | ||
| { | ||
| exception.Should().BeOfType<TaskCanceledException>(); | ||
| } | ||
| else | ||
| { | ||
| exception.Should().BeOfType<OperationCanceledException>(); | ||
| } | ||
| } | ||
|
|
||
| [Theory] | ||
| [InlineData(true, 0, new byte[] { 0, 0 })] | ||
| [InlineData(true, 1, new byte[] { 1, 0 })] | ||
|
|
@@ -267,20 +308,18 @@ public async Task ReadBytes_with_byte_buffer_should_have_expected_effect_for_par | |
| var destination = new ByteArrayBuffer(new byte[3], 3); | ||
| var n = 0; | ||
| var position = 0; | ||
| Task<int> ReadPartial (byte[] buffer, int offset, int count) | ||
| int ReadPartial (byte[] buffer, int offset, int count) | ||
| { | ||
| var length = partition[n++]; | ||
| Buffer.BlockCopy(bytes, position, buffer, offset, length); | ||
| position += length; | ||
| return Task.FromResult(length); | ||
| return length; | ||
| } | ||
|
|
||
| mockStream.Setup(s => s.ReadAsync(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<CancellationToken>())) | ||
| .Returns((byte[] buffer, int offset, int count, CancellationToken cancellationToken) => ReadPartial(buffer, offset, count)); | ||
| mockStream.Setup(s => s.BeginRead(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<AsyncCallback>(), It.IsAny<object>())) | ||
| .Returns((byte[] buffer, int offset, int count, AsyncCallback callback, object state) => ReadPartial(buffer, offset, count)); | ||
| mockStream.Setup(s => s.EndRead(It.IsAny<IAsyncResult>())) | ||
| .Returns<IAsyncResult>(x => ((Task<int>)x).GetAwaiter().GetResult()); | ||
| .Returns((byte[] buffer, int offset, int count, CancellationToken cancellationToken) => Task.FromResult(ReadPartial(buffer, offset, count))); | ||
| mockStream.Setup(s => s.Read(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>())) | ||
| .Returns((byte[] buffer, int offset, int count) => ReadPartial(buffer, offset, count)); | ||
|
|
||
| if (async) | ||
| { | ||
|
|
@@ -533,5 +572,18 @@ private Mock<IByteBuffer> CreateMockByteBuffer(int length) | |
| mockBuffer.SetupGet(b => b.Length).Returns(length); | ||
| return mockBuffer; | ||
| } | ||
|
|
||
| private void SetupStreamRead(Mock<Stream> streamMock, TaskCompletionSource<int> readTaskCompletionSource = null) | ||
| { | ||
| readTaskCompletionSource ??= new TaskCompletionSource<int>(); | ||
| streamMock.Setup(s => s.Close()).Callback(() => | ||
| { | ||
| readTaskCompletionSource.SetException(new IOException()); | ||
| }); | ||
| streamMock.Setup(s => s.Read(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>())).Returns(() => | ||
| readTaskCompletionSource.Task.GetAwaiter().GetResult()); | ||
| streamMock.Setup(s => s.ReadAsync(It.IsAny<byte[]>(), It.IsAny<int>(), It.IsAny<int>(), It.IsAny<CancellationToken>())).Returns(() => | ||
| readTaskCompletionSource.Task); | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.