Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 07b5a17

Browse files
committed
Fix stale async-related comments in FileStream
Leftover after the async rewrite in #972. Fixes #2117.
1 parent fc96be9 commit 07b5a17

File tree

2 files changed

+18
-29
lines changed

2 files changed

+18
-29
lines changed

src/System.IO.FileSystem/src/System/IO/FileStream.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,9 +234,9 @@ public override Task<int> ReadAsync(byte[] buffer, int offset, int count, Cancel
234234
Contract.EndContractBlock();
235235

236236
// If we have been inherited into a subclass, the following implementation could be incorrect
237-
// since it does not call through to Read() or BeginRead() which a subclass might have overridden.
237+
// since it does not call through to Read() or ReadAsync() which a subclass might have overridden.
238238
// To be safe we will only use this implementation in cases where we know it is safe to do so,
239-
// and delegate to our base class (which will call into Read/BeginRead) when we are not sure.
239+
// and delegate to our base class (which will call into Read/ReadAsync) when we are not sure.
240240
if (this.GetType() != typeof(FileStream))
241241
return base.ReadAsync(buffer, offset, count, cancellationToken);
242242

@@ -276,9 +276,9 @@ public override Task WriteAsync(byte[] buffer, int offset, int count, Cancellati
276276
Contract.EndContractBlock();
277277

278278
// If we have been inherited into a subclass, the following implementation could be incorrect
279-
// since it does not call through to Write() or BeginWrite() which a subclass might have overridden.
279+
// since it does not call through to Write() or WriteAsync() which a subclass might have overridden.
280280
// To be safe we will only use this implementation in cases where we know it is safe to do so,
281-
// and delegate to our base class (which will call into Write/BeginWrite) when we are not sure.
281+
// and delegate to our base class (which will call into Write/WriteAsync) when we are not sure.
282282
if (this.GetType() != typeof(FileStream))
283283
return base.WriteAsync(buffer, offset, count, cancellationToken);
284284

src/System.IO.FileSystem/src/System/IO/Win32FileStream.cs

Lines changed: 14 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
/*
1212
* Win32FileStream supports different modes of accessing the disk - async mode
1313
* and sync mode. They are two completely different codepaths in the
14-
* sync & async methods (ie, Read/Write vs. BeginRead/BeginWrite). File
14+
* sync & async methods (ie, Read/Write vs. ReadAsync/WriteAsync). File
1515
* handles in NT can be opened in only sync or overlapped (async) mode,
1616
* and we have to deal with this pain. Stream has implementations of
1717
* the sync methods in terms of the async ones, so we'll
@@ -650,9 +650,8 @@ public override void SetLength(long value)
650650
SetLengthCore(value);
651651
}
652652

653-
// We absolutely need this method broken out so that BeginWriteCore can call
654-
// a method without having to go through buffering code that might call
655-
// FlushWrite.
653+
// We absolutely need this method broken out so that WriteInternalCoreAsync can call
654+
// a method without having to go through buffering code that might call FlushWrite.
656655
[System.Security.SecuritySafeCritical] // auto-generated
657656
private void SetLengthCore(long value)
658657
{
@@ -1109,7 +1108,7 @@ private Task<int> ReadInternalAsync(byte[] array, int offset, int numBytes, Canc
11091108
// OS appears to use a 4K buffer internally. If you write to a
11101109
// pipe that is full, you will block until someone read from
11111110
// that pipe. If you try reading from an empty pipe and
1112-
// Win32FileStream's BeginRead blocks waiting for data to fill it's
1111+
// Win32FileStream's ReadAsync blocks waiting for data to fill it's
11131112
// internal buffer, you will be blocked. In a case where a child
11141113
// process writes to stdout & stderr while a parent process tries
11151114
// reading from both, you can easily get into a deadlock here.
@@ -1194,9 +1193,6 @@ private Task<int> ReadInternalAsync(byte[] array, int offset, int numBytes, Canc
11941193
_readLen = 0;
11951194
return ReadInternalCoreAsync(array, offset + n, numBytes - n, n, cancellationToken);
11961195
}
1197-
// WARNING: all state on asyncResult objects must be set before
1198-
// we call ReadFile in BeginReadCore, since the OS can run our
1199-
// callback & the user's callback before ReadFile returns.
12001196
}
12011197
}
12021198

@@ -1207,7 +1203,7 @@ unsafe private Task<int> ReadInternalCoreAsync(byte[] bytes, int offset, int num
12071203
Debug.Assert(_parent.CanRead, "_parent.CanRead");
12081204
Debug.Assert(bytes != null, "bytes != null");
12091205
Debug.Assert(_writePos == 0, "_writePos == 0");
1210-
Debug.Assert(_isAsync, "BeginReadCore doesn't work on synchronous file streams!");
1206+
Debug.Assert(_isAsync, "ReadInternalCoreAsync doesn't work on synchronous file streams!");
12111207
Debug.Assert(offset >= 0, "offset is negative");
12121208
Debug.Assert(numBytes >= 0, "numBytes is negative");
12131209

@@ -1359,7 +1355,7 @@ private Task WriteInternalAsync(byte[] array, int offset, int numBytes, Cancella
13591355
// OS appears to use a 4K buffer internally. If you write to a
13601356
// pipe that is full, you will block until someone read from
13611357
// that pipe. If you try reading from an empty pipe and
1362-
// Win32FileStream's BeginRead blocks waiting for data to fill it's
1358+
// Win32FileStream's ReadAsync blocks waiting for data to fill it's
13631359
// internal buffer, you will be blocked. In a case where a child
13641360
// process writes to stdout & stderr while a parent process tries
13651361
// reading from both, you can easily get into a deadlock here.
@@ -1404,7 +1400,7 @@ unsafe private Task<int> WriteInternalCoreAsync(byte[] bytes, int offset, int nu
14041400
Debug.Assert(_parent.CanWrite, "_parent.CanWrite");
14051401
Debug.Assert(bytes != null, "bytes != null");
14061402
Debug.Assert(_readPos == _readLen, "_readPos == _readLen");
1407-
Debug.Assert(_isAsync, "BeginWriteCore doesn't work on synchronous file streams!");
1403+
Debug.Assert(_isAsync, "WriteInternalCoreAsync doesn't work on synchronous file streams!");
14081404
Debug.Assert(offset >= 0, "offset is negative");
14091405
Debug.Assert(numBytes >= 0, "numBytes is negative");
14101406

@@ -1416,15 +1412,15 @@ unsafe private Task<int> WriteInternalCoreAsync(byte[] bytes, int offset, int nu
14161412
{
14171413
// Make sure we set the length of the file appropriately.
14181414
long len = _parent.Length;
1419-
//Console.WriteLine("BeginWrite - Calculating end pos. pos: "+pos+" len: "+len+" numBytes: "+numBytes);
1415+
//Console.WriteLine("WriteInternalCoreAsync - Calculating end pos. pos: "+pos+" len: "+len+" numBytes: "+numBytes);
14201416

14211417
// Make sure we are writing to the position that we think we are
14221418
if (_exposedHandle)
14231419
VerifyOSHandlePosition();
14241420

14251421
if (_pos + numBytes > len)
14261422
{
1427-
//Console.WriteLine("BeginWrite - Setting length to: "+(pos + numBytes));
1423+
//Console.WriteLine("WriteInternalCoreAsync - Setting length to: "+(pos + numBytes));
14281424
SetLengthCore(_pos + numBytes);
14291425
}
14301426

@@ -1439,7 +1435,7 @@ unsafe private Task<int> WriteInternalCoreAsync(byte[] bytes, int offset, int nu
14391435
SeekCore(numBytes, SeekOrigin.Current);
14401436
}
14411437

1442-
//Console.WriteLine("BeginWrite finishing. pos: "+pos+" numBytes: "+numBytes+" _pos: "+_pos+" Position: "+Position);
1438+
//Console.WriteLine("WriteInternalCoreAsync finishing. pos: "+pos+" numBytes: "+numBytes+" _pos: "+_pos+" Position: "+Position);
14431439

14441440
int errorCode = 0;
14451441
// queue an async WriteFile operation and pass in a packed overlapped
@@ -1591,15 +1587,12 @@ private unsafe int ReadFileNative(SafeFileHandle handle, byte[] bytes, int offse
15911587
if (r == 0)
15921588
{
15931589
errorCode = Marshal.GetLastWin32Error();
1594-
// We should never ignore an error here without some extra work.
1595-
// We must make sure that BeginReadCore won't return an
1596-
// IAsyncResult that will cause EndRead to block, since the OS
1597-
// won't call AsyncFSCallback for us.
1590+
15981591
if (errorCode == ERROR_BROKEN_PIPE || errorCode == Interop.mincore.Errors.ERROR_PIPE_NOT_CONNECTED)
15991592
{
16001593
// This handle was a pipe, and it's done. Not an error, but EOF.
16011594
// However, the OS will not call AsyncFSCallback!
1602-
// Let the caller handle this, since BeginReadCore & ReadCore
1595+
// Let the caller handle this, since ReadInternalCoreAsync & ReadCore
16031596
// need to do different things.
16041597
return -1;
16051598
}
@@ -1662,16 +1655,12 @@ private unsafe int WriteFileNative(SafeFileHandle handle, byte[] bytes, int offs
16621655
if (r == 0)
16631656
{
16641657
errorCode = Marshal.GetLastWin32Error();
1665-
// We should never ignore an error here without some extra work.
1666-
// We must make sure that BeginWriteCore won't return an
1667-
// IAsyncResult that will cause EndWrite to block, since the OS
1668-
// won't call AsyncFSCallback for us.
16691658

16701659
if (errorCode == ERROR_NO_DATA)
16711660
{
16721661
// This handle was a pipe, and the pipe is being closed on the
1673-
// other side. Let the caller handle this, since BeginWriteCore
1674-
// & WriteCore need to do different things.
1662+
// other side. Let the caller handle this, since Write
1663+
// and WriteAsync need to do different things.
16751664
return -1;
16761665
}
16771666

0 commit comments

Comments
 (0)