RFR 8023173: FileOutputStream(FileDescriptor) does not respect append flag on Windows

[Ivan Gerasimov]

Thank you Alan for the review!

On 17.10.2014 13:00, Alan Bateman wrote:
> On 06/10/2014 11:41, Ivan Gerasimov wrote:
>> Hello everybody!
>>
>> The append mode is emulated on Windows, therefore we have to keep a 
>> flag indicating that.
>>
>> With the current implementation, the FileDescriptor does not know if 
>> the file were opened with O_APPEND, and the flag is maintained at the 
>> upper level.
>> This can cause inconsistency, when the FileDescriptor is reused to 
>> construct a new FileOutputStream, as there is no information 
>> available about the append/non-append mode.
>>
>> Even though the solution is quite straight-forward: moving the flag 
>> from FileOutputStream,  FileDispatcherImpl and FileChannelImpl to the 
>> lower level of FileDescriptor, it touches 20 files across the 
>> source-code base.
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8023173
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8023173/0/webrev/
>>
>> With the fix, all the io/nio tests, including the new one, pass on 
>> all available platforms.
>>
>> Would you please help review this fix?
> Martin prodded me a few times but I was busy with other things and 
> only getting to this now.
>
> At a high level then moving the append down to FileDescriptor make 
> sense but I have a few concerns.
>
> On Unix systems then it looks like getAppend is calling into the ioctl 
> to get the file descriptor flags.
> If I read it correctly then it creates several problems for the usage 
> in FileChannelImpl.position.

I thought it might look like a data duplication, if we store the append 
flag both in FileChannelImpl and FileDescriptor.
Though, we surely can retrieve the append flag only once and cache it.

I updated the webrev with this change.

> I just wondering if you consider just leaving the append flag there 
> and just retrieve the value once in the open method.
>

> I'm also wondering about the handleWrite implementation on Windows 
> which changes to use additional JNI to retrieve the value of the 
> append flag each time. We should try to avoid this (we want the native 
> methods to be as simple as possible so that we can replace them in the 
> future) so there may be an argument for passing that down as per the 
> current implementation.
>

We would still need to retrieve the flag from native code.
For example, the platform specific handleWrite() is called from the 
native FileOutputStream.writeBytes(), which is called from 
FileOutputStream.writt(), which is common for all the platforms.
Thus, we should either retrieve the append flag from the Java code for 
any platform, and ignore it later on Unix, or read the flag in the 
native Windows-only code.

Alternatively, we could separate implementations of 
FileOutputStream.java for different platforms, but that would complicate 
everything.

Hopefully, in the future we could find a way to use FILE_APPEND_DATA, so 
FileDescriptor.append can be removed altogether with that corresponding 
JNI code.

Here's the updated webrev:
http://cr.openjdk.java.net/~igerasim/8023173/1/webrev/

I've also included a few typo fixes left after JDK-8059840.

Sincerely yours,
Ivan