6709457: (fc) lock/tryLock() throws IOException "Access is denied" when file opened for append [win]
chris.hegarty at oracle.com
Wed Dec 1 11:54:58 UTC 2010
Alan, I think the changes are fine.
On 29/11/2010 11:25, Alan Bateman wrote:
> This is an awkward regression that Martin and I previously discussed on
> this list .
> To re-cap: When the redirection API for sub-processes was added early in
> jdk7, FileOutputStream was changed on Windows so that opening a file in
> append mode results in the write-at-end to be atomic. It achieves this
> by opening the file with FILE_APPEND_DATA and not FILE_WRITE_DATA
> access. Unfortunately this change came with side effects, the main one
> being that it breaks the file locking facility for files opened for
> append. We also observed that files opened for append cannot be
> truncated, potentially an issue for code that wants to truncate a log
> file for example.
> The changes here are along the lines that we discussed. First, the
> opening of files for append is changed back to using GENERIC_WRITE mode.
> Secondly, writing is changed so that it does a write-at-end. This is
> probably the way it should have been done originally. For the
> redirection API, the Windows implementation is changed so that it opens
> the output file in FILE_APPEND_DATA mode. This gives us the semantics
> that we want as we have no control as to how the sub-process will write
> to its output or error streams. One thing to point out is that I have
> deliberately kept the code as simple as possible. That is, I have not
> duplicated the code for long file handling and all the other odd cases
> that Windows entails. The rational is that we could could easily just
> eliminate this and have it use the new file system API to open the file
> (as supporting platform specific options there is easy). However, there
> are modularity concerns with creating that dependency and it might be
> better to see how that plays out first. We can create a low priority bug
> as a reminder of this odd cases. That's mostly it except for some
> additional tests for these scenarios.
> The webrev with the changes is here:
More information about the core-libs-dev