Skip to content

Commit cdc7e17

Browse files
[release/7.0] Invalidate buffered data when bypassing the cache (#80503)
* add a failing test * fix * address code review feedback * test BufferedStream as well Co-authored-by: Adam Sitnik <[email protected]>
1 parent cb2d02e commit cdc7e17

File tree

2 files changed

+60
-3
lines changed

2 files changed

+60
-3
lines changed

src/libraries/System.IO.FileSystem/tests/FileStream/ReadAsync.cs

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ namespace System.IO.Tests
1111
{
1212
public abstract class FileStream_AsyncReads : FileSystemTest
1313
{
14-
protected abstract Task<int> ReadAsync(FileStream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken = default);
14+
protected abstract Task<int> ReadAsync(Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken = default);
1515

1616
[Fact]
1717
public async Task EmptyFileReadAsyncSucceedSynchronously()
@@ -132,17 +132,72 @@ public async Task IncompleteReadCantSetPositionBeyondEndOfFile(FileShare fileSha
132132
Assert.Equal(fileSize, fs.Position);
133133
}
134134
}
135+
136+
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
137+
[InlineData(true, true)]
138+
[InlineData(true, false)]
139+
[InlineData(false, false)]
140+
[InlineData(false, true)]
141+
public async Task BypassingCacheInvalidatesCachedData(bool fsIsAsync, bool asyncReads)
142+
{
143+
const int BufferSize = 4096;
144+
const int FileSize = BufferSize * 4;
145+
string filePath = GetTestFilePath();
146+
byte[] content = RandomNumberGenerator.GetBytes(FileSize);
147+
File.WriteAllBytes(filePath, content);
148+
149+
await Test(new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, BufferSize, fsIsAsync));
150+
await Test(new BufferedStream(new FileStream(filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite, 0, fsIsAsync), BufferSize));
151+
152+
async Task Test(Stream stream)
153+
{
154+
try
155+
{
156+
// 1. Populates the private stream buffer, leaves bufferSize - 1 bytes available for next read.
157+
await ReadAndAssertAsync(stream, 1);
158+
// 2. Consumes all available data from the buffer, reads another bufferSize-many bytes from the disk and copies the 1 missing byte.
159+
await ReadAndAssertAsync(stream, BufferSize);
160+
// 3. Seek back by the number of bytes consumed from the buffer, all buffered data is now available for next read.
161+
stream.Position -= 1;
162+
// 4. Consume all buffered data.
163+
await ReadAndAssertAsync(stream, BufferSize);
164+
// 5. Bypass the cache (all buffered data has been consumed and we need bufferSize-many bytes).
165+
// The cache should get invalidated now!!
166+
await ReadAndAssertAsync(stream,BufferSize);
167+
// 6. Seek back by just a few bytes.
168+
stream.Position -= 9;
169+
// 7. Perform a read, which should not use outdated buffered data.
170+
await ReadAndAssertAsync(stream,BufferSize);
171+
}
172+
finally
173+
{
174+
await stream.DisposeAsync();
175+
}
176+
}
177+
178+
async Task ReadAndAssertAsync(Stream stream, int size)
179+
{
180+
var initialPosition = stream.Position;
181+
var buffer = new byte[size];
182+
183+
var count = asyncReads
184+
? await ReadAsync(stream, buffer, 0, size)
185+
: stream.Read(buffer);
186+
187+
Assert.Equal(content.Skip((int)initialPosition).Take(count), buffer.Take(count));
188+
}
189+
}
135190
}
136191

137192
public class FileStream_ReadAsync_AsyncReads : FileStream_AsyncReads
138193
{
139-
protected override Task<int> ReadAsync(FileStream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
194+
protected override Task<int> ReadAsync(Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
140195
stream.ReadAsync(buffer, offset, count, cancellationToken);
141196
}
142197

143198
public class FileStream_BeginEndRead_AsyncReads : FileStream_AsyncReads
144199
{
145-
protected override Task<int> ReadAsync(FileStream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
200+
protected override Task<int> ReadAsync(Stream stream, byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
146201
Task.Factory.FromAsync(
147202
(callback, state) => stream.BeginRead(buffer, offset, count, callback, state),
148203
iar => stream.EndRead(iar),

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,8 @@ public override ValueTask<int> ReadAsync(Memory<byte> buffer, CancellationToken
326326
{
327327
if (_readLen == _readPos && buffer.Length >= _bufferSize)
328328
{
329+
// invalidate the buffered data, otherwise certain Seek operation followed by a ReadAsync could try to re-use data from _buffer
330+
_readPos = _readLen = 0;
329331
// hot path #1: the read buffer is empty and buffering would not be beneficial
330332
// To find out why we are bypassing cache here, please see WriteAsync comments.
331333
return _strategy.ReadAsync(buffer, cancellationToken);

0 commit comments

Comments
 (0)