[PATCH] Initialize IO_STATUS_BLOCK for pread, pwrite

[Mark Geisert]

Corinna Vinschen wrote:
> On Nov 28 00:03, Mark Geisert wrote:
>> Mark Geisert wrote:
>>> ---
>>>  winsup/cygwin/fhandler_disk_file.cc | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/winsup/cygwin/fhandler_disk_file.cc b/winsup/cygwin/fhandler_disk_file.cc
>>> index 5dfcae4d9..2ead9948c 100644
>> [...]
>>
>> Oops, I neglected to include an explanatory comment. Issuing simultaneous
>> pwrite(s) on one file descriptor from multiple threads, as one might do in a
>> forthcoming POSIX aio implementation, sometimes results in garbage status in
>> the IO_STATUS_BLOCK on return from NtWriteFile(). Zeroing beforehand made
>> the issue go away.
>>
>> This is mildly concerning to me because there are many other uses of
>> IO_STATUS_BLOCK in the Cygwin DLL that haven't seemed to have needed
>> initialization.
>>
>> Puzzledly,
>
> Ok, let's start with, why did you tweak pread if you only observed
> a problem in pwrite?

Optimism? :-)  No, you're correct; I was getting ahead of myself.

>                       In terms of pread, we already have a very recent
> patch series:
>
> https://sourceware.org/git/?p=newlib-cygwin.git;a=commitdiff;h=46702f92ea49
> https://sourceware.org/git/?p=newlib-cygwin.git;a=commitdiff;h=c983aa48798d
> https://sourceware.org/git/?p=newlib-cygwin.git;a=commitdiff;h=181fe5d2edac
>
> In this case it turned out that the problem was related to hitting EOF.
> I wonder if we hit a similar problem here.
>
> Two points:
>
> - Did you check the status code returned by NtWriteFile?  Not all non-0
>   status codes fail the !NT_SUCCESS (status) test.

I did check the status code but don't recall what it was.  The symptom I was 
seeing was outlandish io.Information values being returned by pwrite().  Far 
larger than the number requested in the call to pwrite() and NtWriteFile().

>
> - Do you have a simple, self-contained testcase?

That would be difficult.  I can supply an strace excerpt just showing the region 
of these simultaneous pwrite() calls, without this patch.  If it's too large 
I'll put it somewhere and post a link (but I don't think it will be).

..mark