TrackingFileManager changes #1692
Conversation
Drop out of band deletion, let TFM delete as well. Also, simplify but do not drop locking, is needed for interoperability with 3.9.11 and 4.0.0-rc-5 and older released versions.
|
@bade7n ping |
If you'd like me to test it, feel free to release an alpha or beta build — I can provide feedback on 1.9 within a few days. |
|
|
| } | ||
| synchronized (mutex(path)) { | ||
| try (FileChannel fileChannel = FileChannel.open(path, StandardOpenOption.READ); | ||
| FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), true)) { |
There was a problem hiding this comment.
what's the purpose of using fileLock(fileChannel, x) ? Why not using fileLock(fileChannel, 0) ?
According to the javadoc:
A value of zero means to lock all bytes from the specified starting position to the end of the file, regardless of whether the file is subsequently extended or truncated
There was a problem hiding this comment.
Basically we lock with size 0 or 1 (so all or [0..1] bytes), based on file was just created or existed.
There was a problem hiding this comment.
Yes, that's what the code does. I don't understand why. What we want is a lock on the full file, so why just requesting 1 byte instead of the whole length (which will be extended automatically when written) ?
This parameter seems useless to me.
There was a problem hiding this comment.
And it forces to add a call to retrieve the file size...
There was a problem hiding this comment.
I think you looked at outdated PR? As this morning I removed the 0/1 dance fully (and hence the file size call as well)
| try (FileChannel fileChannel = FileChannel.open( | ||
| path, StandardOpenOption.READ, StandardOpenOption.WRITE, StandardOpenOption.CREATE); | ||
| FileLock unused = fileLock(fileChannel, Math.max(1, fileChannel.size()), false)) { | ||
| if (fileChannel.size() > 0) { |
There was a problem hiding this comment.
why is there a check here and not while reading ?
There was a problem hiding this comment.
Reading will be unable to open absent file, here we may create it (and will be 0 byte)
| fileSize = Files.size(path); | ||
| } catch (IOException e) { | ||
| fileSize = 0L; | ||
| Properties props = new Properties(); |
There was a problem hiding this comment.
The props variable should be defined inside the try block, with the return statement moved at the end of the same block.
Backport from master. Needs Maven changes as well. Aligned with master fully. Master PR: #1692
3 participants
Drop out of band deletion, let TFM delete as well. Also, simplify but do not drop locking, is needed for interoperability with 3.9.11 and 4.0.0-rc-5 and older released versions. Make FNFEx ignored in case of read/delete.
Backport to 1.9.x is here #1695.