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
