ByteBufferCleaner to check Unsafe access in Java 23+ - #866
Conversation
In Java 23+, ByteBufferCleaner should check for access to deprecated memory-access methods in sun.misc.Unsafe
|
Converting to draft until I can have a closer look, needs clean ups anyway. |
garydgregory
left a comment
There was a problem hiding this comment.
There are several problems here:
- The build fails
- Using
java.lang.managementadds a new module dependency which some people will complain about. We could disallow this through Checkstyle but we currently do not. - It's not clear to me this is the correct implementation since the JEP is just a deprecation.
- There is a bug where
clearWritable()is not called.
fixed
we just disable Unsafe access on Java 23+ now
deprecation is for removal. Plus, most users would want the library to not trigger scary warnings.
fixed |
|
@maxxedev The builds for this PR have been broken since last week. |
|
@garydgregory please review again :) |
|
Hello @maxxedev, Thank you for updating this PR. I don't think this is acceptable as-is because it now violates the write once, run anywhere Java principle: the behavior changes depending on which Java version the application is running on, with no way for the user to control or opt out of that change. I think that is particularly problematic for a deprecated API; the fact that an API is deprecated should not mean its behavior changes depending on the runtime version. If you'd like to pursue this change further, I suggest making the new behavior explicitly opt-in. That would preserve the existing behavior by default while still allowing users who want the new behavior to enable it. |
…d MemoryMappedFileInputStream. if true (default), then attempt to clean ByteBuffer
Right now on JDK23+, this warning is shown: That's JDK telling the library to change the behavior by not using Unsafe on JDK23+. commons-io library should be a good citizen and automatically follow the suggestion set by JDK. |
In Java 23+, ByteBufferCleaner should check for access to deprecated memory-access methods in sun.misc.Unsafe
mvn; that'smvnon the command line by itself.