Skip to content
Closed
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
117 changes: 117 additions & 0 deletions src/libraries/System.IO/tests/Stream/Stream.NullTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ public static void TestNullStream_Dispose()
Stream.Null.Dispose(); // Dispose shouldn't have any side effects
}

[Fact]
public static async Task TestNullStream_DisposeAsync()
{
await Stream.Null.DisposeAsync();
await Stream.Null.DisposeAsync(); // Dispose shouldn't have any side effects
}

[Fact]
public static async Task TestNullStream_CopyTo()
{
Expand Down Expand Up @@ -133,6 +140,16 @@ public static void TestNullTextReaderDispose(TextReader input)
Assert.Equal("", input.ReadToEnd());
}

[Theory]
[MemberData(nameof(NullReaders))]
public static async Task TestNullTextReaderDisposeAsync(TextReader input)
{
// dispose should be a no-op
await input.DisposeAsync();
await input.DisposeAsync();
Assert.Equal("", input.ReadToEnd());
}

[Theory]
[MemberData(nameof(NullReaders))]
public static void TestNullTextReader(TextReader input)
Expand Down Expand Up @@ -172,6 +189,22 @@ public static async Task TestNullTextReaderAsync(TextReader input)
input.Dispose();
}

[Theory]
[MemberData(nameof(NullReaders))]
public static async Task TestNullTextReaderAsync_DisposeAsync(TextReader input)
{
var chars = new char[2];
Assert.Equal(0, await input.ReadAsync(chars, 0, chars.Length));
Assert.Equal(0, await input.ReadAsync(chars.AsMemory(), default));
Assert.Equal(0, await input.ReadBlockAsync(chars, 0, chars.Length));
Assert.Equal(0, await input.ReadBlockAsync(chars.AsMemory(), default));
Assert.Null(await input.ReadLineAsync());
Assert.Null(await input.ReadLineAsync(default));
Assert.Equal("", await input.ReadToEndAsync());
Assert.Equal("", await input.ReadToEndAsync(default));
await input.DisposeAsync();
Copy link
Member

Choose a reason for hiding this comment

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

Can you please add this line to the existing test above and remove this one test to avoid duplication?

Copy link
Author

@dersia dersia Jul 29, 2023

Choose a reason for hiding this comment

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

I think these to methods are testing different behavior, since Dispose is calling "Close()" on the underlying stream and DisposeAsync() is calling "DisposeAsync()" on the underlying stream. But if this isn't a concern I will happily change this.

Copy link
Author

Choose a reason for hiding this comment

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

@jozkee what do you think we should do here?

Copy link
Member

Choose a reason for hiding this comment

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

I still think you should apply my suggestion here because TestNullTextReaderAsync above is already testing Async conterparts.

}

[Theory]
[MemberData(nameof(NullReaders))]
public static async Task TestCanceledNullTextReaderAsync(TextReader input)
Expand All @@ -192,6 +225,76 @@ public static async Task TestCanceledNullTextReaderAsync(TextReader input)
input.Dispose();
}

[Theory]
[MemberData(nameof(NullReaders))]
public static async Task TestCanceledNullTextReaderAsync_DisposeAsync(TextReader input)
{
using CancellationTokenSource tokenSource = new CancellationTokenSource();
tokenSource.Cancel();
var token = tokenSource.Token;
var chars = new char[2];
OperationCanceledException ex;
ex = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await input.ReadAsync(chars.AsMemory(), token));
Assert.Equal(token, ex.CancellationToken);
ex = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await input.ReadBlockAsync(chars.AsMemory(), token));
Assert.Equal(token, ex.CancellationToken);
ex = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await input.ReadLineAsync(token));
Assert.Equal(token, ex.CancellationToken);
ex = await Assert.ThrowsAnyAsync<OperationCanceledException>(async () => await input.ReadToEndAsync(token));
Assert.Equal(token, ex.CancellationToken);
await input.DisposeAsync();
Copy link
Member

Choose a reason for hiding this comment

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

Same here.

}

[Theory]
[MemberData(nameof(NullReaders))]
public static void TestNullTextReader_Disposed(TextReader input)
{
StreamReader sr = input as StreamReader;

input.Dispose();

if (sr != null)
Assert.True(sr.EndOfStream, "EndOfStream property didn't return true");
Assert.Null(input.ReadLine());
if (sr != null)
Assert.True(sr.EndOfStream, "EndOfStream property didn't return true");

Assert.Equal(-1, input.Read());
Assert.Equal(-1, input.Peek());
var chars = new char[2];
Assert.Equal(0, input.Read(chars, 0, chars.Length));
Assert.Equal(0, input.Read(chars.AsSpan()));
Assert.Equal(0, input.ReadBlock(chars, 0, chars.Length));
Assert.Equal(0, input.ReadBlock(chars.AsSpan()));
Assert.Equal("", input.ReadToEnd());
input.Dispose();
}

[Theory]
[MemberData(nameof(NullReaders))]
public static async Task TestNullTextReader_DisposedAsync(TextReader input)
{
StreamReader sr = input as StreamReader;

await input.DisposeAsync();

if (sr != null)
Assert.True(sr.EndOfStream, "EndOfStream property didn't return true");
Assert.Null(input.ReadLine());
if (sr != null)
Assert.True(sr.EndOfStream, "EndOfStream property didn't return true");

Assert.Equal(-1, input.Read());
Assert.Equal(-1, input.Peek());
var chars = new char[2];
Assert.Equal(0, await input.ReadAsync(chars, 0, chars.Length));
Assert.Equal(0, input.Read(chars.AsSpan()));
Assert.Equal(0, await input.ReadBlockAsync(chars, 0, chars.Length));
Assert.Equal(0, input.ReadBlock(chars.AsSpan()));
Assert.Equal("", await input.ReadToEndAsync());
await input.DisposeAsync();
}

[Theory]
[MemberData(nameof(NullWriters))]
public static void TextNullTextWriter(TextWriter output)
Expand All @@ -206,6 +309,20 @@ public static void TextNullTextWriter(TextWriter output)
output.Dispose();
}

[Theory]
[MemberData(nameof(NullWriters))]
public static async Task TextNullTextWriterAsync(TextWriter output)
{
await output.FlushAsync();
await output.DisposeAsync();

await output.WriteLineAsync(decimal.MinValue.ToString());
await output.WriteLineAsync(Math.PI.ToString());
await output.WriteLineAsync(output.NewLine);
await output.FlushAsync();
await output.DisposeAsync();
}

[Theory]
[MemberData(nameof(NullStream_ReadWriteData))]
public void TestNullStream_ReadSpan(byte[] buffer, int offset, int count)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Text;
using System.Threading.Tasks;
using Xunit;

namespace System.IO.Tests
{
public class StreamReader_disposeAsyncTest
{

[Fact]
public void DisposeAsync_CanInvokeMultipleTimes()
{
var ms = new MemoryStream();
var sr = new StreamReader(ms);
Assert.True(sr.DisposeAsync().IsCompletedSuccessfully);
Assert.True(sr.DisposeAsync().IsCompletedSuccessfully);
}

[Fact]
public void DisposeAsync_CanDisposeAsyncAfterDispose()
{
var ms = new MemoryStream();
var sr = new StreamReader(ms);
sr.Dispose();
Assert.True(sr.DisposeAsync().IsCompletedSuccessfully);
}

[Fact]
public async Task DisposeAsync_LeaveOpenTrue_LeftOpen()
{
var ms = new MemoryStream();
var sr = new StreamReader(ms, Encoding.ASCII, false, 0x1000, leaveOpen: true);
await sr.DisposeAsync();
Assert.Equal(0, ms.Position); // doesn't throw
}

[Fact]
public async Task DisposeAsync_DerivedTypeForcesDisposeToBeUsedUnlessOverridden()
{
var ms = new MemoryStream();
var sr = new OverrideDisposeStreamReader(ms);
Assert.False(sr.DisposeInvoked);
await sr.DisposeAsync();
Assert.True(sr.DisposeInvoked);
}

[Fact]
public async Task DisposeAsync_DerivedTypeDisposeAsyncInvoked()
{
var ms = new MemoryStream();
var sr = new OverrideDisposeAndDisposeAsyncStreamReader(ms);
Assert.False(sr.DisposeInvoked);
Assert.False(sr.DisposeAsyncInvoked);
await sr.DisposeAsync();
Assert.False(sr.DisposeInvoked);
Assert.True(sr.DisposeAsyncInvoked);
}

private sealed class OverrideDisposeStreamReader : StreamReader
{
public bool DisposeInvoked;
public OverrideDisposeStreamReader(Stream output) : base(output) { }
protected override void Dispose(bool disposing) => DisposeInvoked = true;
}

private sealed class OverrideDisposeAndDisposeAsyncStreamReader : StreamReader
{
public bool DisposeInvoked, DisposeAsyncInvoked;
public OverrideDisposeAndDisposeAsyncStreamReader(Stream output) : base(output) { }
protected override void Dispose(bool disposing) => DisposeInvoked = true;
public override ValueTask DisposeAsync() { DisposeAsyncInvoked = true; return default; }
}
}
}
1 change: 1 addition & 0 deletions src/libraries/System.IO/tests/System.IO.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
<Compile Include="MemoryStream\MemoryStreamTests.cs" />
<Compile Include="StreamReader\StreamReader.CtorTests.cs" />
<Compile Include="StreamReader\StreamReaderTests.cs" />
<Compile Include="StreamReader\StreamReader.DisposeAsync.cs" />
<Compile Include="StreamReader\StreamReaderTests_Serial.cs" />
<Compile Include="StreamWriter\StreamWriter.BaseStream.cs" />
<Compile Include="StreamWriter\StreamWriter.CloseTests.cs" />
Expand Down
26 changes: 26 additions & 0 deletions src/libraries/System.IO/tests/TextReader/TextReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -310,5 +310,31 @@ public void ReadBlockSpan()
}
}
}

[Fact]
public void DisposeAsync_InvokesDisposeSynchronously()
{
bool disposeInvoked = false;
var tw = new InvokeActionOnDisposeTextReader() { DisposeAction = () => disposeInvoked = true };
Assert.False(disposeInvoked);
Assert.True(tw.DisposeAsync().IsCompletedSuccessfully);
Assert.True(disposeInvoked);
}

[Fact]
public void DisposeAsync_ExceptionReturnedInTask()
{
Exception e = new FormatException();
var tw = new InvokeActionOnDisposeTextReader() { DisposeAction = () => { throw e; } };
ValueTask vt = tw.DisposeAsync();
Assert.True(vt.IsFaulted);
Assert.Same(e, vt.AsTask().Exception.InnerException);
}

private sealed class InvokeActionOnDisposeTextReader : TextReader
{
public Action DisposeAction;
protected override void Dispose(bool disposing) => DisposeAction?.Invoke();
}
}
}
71 changes: 66 additions & 5 deletions src/libraries/System.Private.CoreLib/src/System/IO/StreamReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -245,29 +245,90 @@ protected override void Dispose(bool disposing)
{
return;
}
_disposed = true;
try
{
if (disposing)
{
CheckAsyncTaskInProgress();
}
}
finally
{
CloseStreamFromDispose(disposing);
}
}

// Dispose of our resources if this StreamReader is closable.
if (_closable)
private void CloseStreamFromDispose(bool disposing)
{
// Dispose of our resources if this StreamWriter is closable.
if (_closable && !_disposed)
{
try
{
// Note that Stream.Close() can potentially throw here. So we need to
// ensure cleaning up internal resources, inside the finally block.
// Attempt to close the stream even if there was an IO error from Flushing.
// Note that Stream.Close() can potentially throw here (may or may not be
// due to the same Flush error). In this case, we still need to ensure
// cleaning up internal resources, hence the finally block.
if (disposing)
{
_stream.Close();
}
}
finally
{
_disposed = true;
_charPos = 0;
_charLen = 0;
base.Dispose(disposing);
}
}
}

public override ValueTask DisposeAsync() =>
GetType() != typeof(StreamReader) ?
base.DisposeAsync() :
DisposeAsyncCore();

private async ValueTask DisposeAsyncCore()
{
// Same logic as in StreamWriter.DisposeAsync().
Debug.Assert(GetType() == typeof(StreamReader));
try
{
if (!_disposed)
{
CheckAsyncTaskInProgress();
}
}
finally
{
await CloseStreamFromDisposeAsync().ConfigureAwait(false);
}
GC.SuppressFinalize(this);
}
private async ValueTask CloseStreamFromDisposeAsync()
{
// Dispose of our resources if this StreamWriter is closable.
if (_closable && !_disposed)
{
try
{
// Attempt to close the stream even if there was an IO error from Flushing.
// Note that Stream.DisposeAsync() can potentially throw here (may or may not be
// due to the same Flush error). In this case, we still need to ensure
// cleaning up internal resources, hence the finally block.
await _stream.DisposeAsync().ConfigureAwait(false);
}
finally
{
_disposed = true;
_charPos = 0;
_charLen = 0;
await base.DisposeAsync().ConfigureAwait(false);
}
}
}

public virtual Encoding CurrentEncoding => _encoding;

public virtual Stream BaseStream => _stream;
Expand Down
Loading