What are the potentially-erroneous results if you don’t pass NULL as the lpNumberOfBytesRead when issuing overlapped I/O?
The documentation for many I/O functions that read or write bytes recommend that you pass NULL as the lpNumberÂOfÂBytesÂRead parameter when issuing overlapped I/O to avoid “potentially erroneous results”. What are these potentially erroneous results the documentation is trying to warn against?
For the purpose of this discussion, let’s use ReadFile as our example, even though the same argument applies to WriteÂFile, WSASend, and other functions which follow the same pattern.
The race condition here is a race between the code that calls ReadFile and the code that handles the asynchronous completion. If the variable passed as the output for ReadFile‘s lpNumberÂOfÂBytesÂRead parameter is the same variable used as the output for GetÂOverlappedÂResult‘s lpNumberÂOfÂBytesÂTransferred parameter, then there is a race because the completion might run concurrently with the exit out of ReadÂFile.
| Thread 1 | Thread 2 |
ReadFile(..., &byteCount, ...); |
|
ReadFile begins |
|
| Â Â I/O initiated asynchronously | |
| Â | I/O completes asynchronously |
| Â | GetÂOverlappedÂResult(..., &byteCount, ...) |
| Â | GetÂOverlappedÂResult sets byteCount = result |
  set byteCount = 0 |
|
  SetLastError(ERROR_IO_PENDING); |
|
  return FALSE; |
If the I/O completes very quickly, then the completion routine can run before ReadÂFile returns. And then when ReadÂFile tries to report the fact that the I/O was initiated asynchronously, it overwrites the byteCount that the completion routine had calculated.
So it’s okay to pass a non-null lpNumberOfBytesRead to ReadÂFile, even when issuing asynchronous I/O, provided that you do so into a different variable from the one that the completion routine uses.
Normally, however, there’s no reason to pass a non-null lpNumberOfBytesRead because the result of the operation is going to be handled by the completion function. But there’s a case where it is advantageous to use a non-null value, and that’s where you have used SetÂFileÂCompletionÂNotificationÂModes to configure the handle as FILE_. In that case, a synchronous completion does not queue a call to the completion function on the I/O completion thread. Instead, the code that called ReadÂFile is expected to deal with the synchronous completion inline. And one of the things it probably wants to know is how many bytes were read, so it would normally call GetÂOverlappedÂResult to find out. You can avoid that extra call to GetÂOverlappedÂResult by passing a non-null pointer to ReadÂFile so that in the case of a synchronous completion, you have your answer immediately.
This is admittedly a micro-optimization. One of my colleagues was not aware of this trick and just followed the guidance in the documentation by passing NULL and calling GetÂOverlappedÂResult, and he says that his code can still stream data at 100Gbps over the network despite doing things “inefficiently”.
So maybe you’re better off not knowing and just following the simple rule of “Use NULL when issuing asynchronous I/O.” It’s easier to explain and easier to remember.
Light
Dark
7 comments
Very odd behavior of ReadFile. I would have expected if it’s going to treat lpNumberOfBytesWritten as an out parameter you would have *lpNumberOfBytesWritten = 0 near the top of the function and just not write to it again if it’s going down the asynchronous pathway.
The documentation of GetOverlappedResult defines the behavior of GetOverlappedResult only for the case that ReadFile (etc.) failed with ERROR_IO_PENDING. If lpNumberOfBytesRead of ReadFile is NULL and ReadFile manages to read synchronously and thus does not fail with ERROR_IO_PENDING and GetOverlappedResult cannot be called in that case: how do I get the number of bytes read? I guess the documentation of GetOverlappedResult is wrong (or I’m not reading it correctly). See also https://devblogs.microsoft.com/oldnewthing/20140206-00/?p=1853
The documentation of ReadFile says:
lpNumberOfBytesRead {…] ReadFile sets this value to zero *before* doing any work or error checking
(Emphasis mine.) Either the documentation is wrong or your sketch of the ReadFile internals is wrong. Why should ReadFile set that value to zero again before returning?
Side question:
I’ve been waiting for a I/O related post to plug a question regarding CopyFileEx / COPY_FILE_OPEN_SOURCE_FOR_WRITE. 😇
https://stackoverflow.com/questions/75388794/what-is-the-purpose-of-copyfileexs-copy-file-open-source-for-write
(I’d have commented on the very fitting “How can I perform a CopyFile, but also …” post https://devblogs.microsoft.com/oldnewthing/20221007-00/?p=107261 from last Oct, but “comments are closed” there.)
Does anybody know what the actual use case (not the implemented behavior) of COPY_FILE_OPEN_SOURCE_FOR_WRITE is?
It’s to support MoveFileWithProgress, specifically when the file being moved is moving across volumes and is the target of a shortcut. There’s a whole infrastructure for shortcut target tracking (see Distributed Link Tracking Service.) It works, in part, by having object IDs on the shortcut target. Once a file is “moved” to a new volume, this information is removed from the source of the move and applied to the target of the move, then the source is deleted. Removing from the source is a modification. The flag is just due to how the code is structured, where copy opens the handles.
Malcom, thanks a bunch.
So, if I understand you correctly, the MoveFileWithProgress shenanigans will use this flag to have CopyFileEx *try* to open the source file in a writable way so that its callback handler can potentially do some modifying operations on the source handle once it gets to the finished state?
So the general use case could be phrased: Use this flag if your callback handler wants to modify something on the source file. But there’s no guarantee (that is no failure on the side CopyFileEx itself) that the source file will be opened in write mode, it’s just best effort.
Yeah, kinda. When the code was written, it performed this update in the callback handler, so depended on copy to open handles for write. It doesn’t do that anymore, but that’s still the reason the flag was added.
As far as the way it “tries” to open the handles for write, see also MOVEFILE_FAIL_IF_NOT_TRACKABLE. Since the case for this is very narrow, most of the time write access isn’t required, so downgrading to read only is completely benign. MoveFile can discover that it doesn’t have write access and fail later for this specific case, although by that point, it’s already copied the data.
So the remaining public use for this is exactly as you describe: you can update the source in a callback handler, but must be prepared to detect access denied conditions and perform appropriate error handling at the time.