RFR [8058099] (ch) Cleanup in FileChannel/FileDispatcher native implementation [win]

[Ivan Gerasimov]

On 22.09.2014 16:09, Alan Bateman wrote:
> On 22/09/2014 10:22, Ivan Gerasimov wrote:
>> :
>>
>>> With the changes to pread0 and pwrite0 then we are using an 
>>> OVERLAPPED structure to specify the position to ReadFile/WriteFile. 
>>> Are we guaranteed that these will always execute synchronously and 
>>> that ERROR_IO_PENDING will not be returned?
>>>
>> According to MSDN [1, 2], the last error code ERROR_IO_PENDING may 
>> only be set if the file was opened with FILE_FLAG_OVERLAPPED. This is 
>> not our case here.
>>
>> Here's the quotation from the ReadFile doc page:
>> "Considerations for working with synchronous file handles:   . . .
>> If lpOverlapped is not NULL, the read operation starts at the offset 
>> that is specified in the OVERLAPPED structure and ReadFile does not 
>> return until the read operation is complete."
>> And the same holds for WriteFile().
>>
>> The only non-obvious effect of using the OVERLAPPED structure with a 
>> synchronous access was testing for EOF.
>> According to MSDN [3], synchronous read should return TRUE upon 
>> reaching EOF, setting number of bytes read to zero.
>>
>> In the real life, when the synchronous read is performed with 
>> non-null pointer to OVERLAPPED structure, the ReadFile function 
>> behaves in the same way as for asynchronous access: it returns FALSE 
>> and sets the last error to ERROR_HANDLE_EOF.
>>
>> This is why I had to add the code at line 162:
>> 162         if (error != ERROR_HANDLE_EOF) { . . .
>
> Martin Doerr brought up an issue recently where LockFileEx was 
> returning ERROR_IO_PENDING [1]. I'm still wondering about that one 
> because I also believed that if we aren't using FILE_FLAG_OVERLAPPED 
> that it would also complete synchronously. Maybe we need to look at 
> that one again to understand how this is possible.
>
That looks scary!
However, I didn't find the reproducer in the thread, so it's hard to say 
how it was possible to make it fail.
I can revert these two usages of OVERLAPPED to be safe.
On the other hand, we have a few more places where OVERLAPPED structure 
is used, but the action is assumed to complete synchronously - i.e. no 
checks for ERROR_IO_PENDING error are performed.
The places are write0 and writev0 (in append mode) and release0.

Shouldn't we check for ERROR_IO_PENDING more consistently?

Sincerely yours,
Ivan


> -Alan
>
> [1] http://mail.openjdk.java.net/pipermail/nio-dev/2014-July/002687.html
>
>