xazap opened a new pull request, #897: URL: https://github.com/apache/commons-io/pull/897
This PR seeks to fix the bug described in IO-895. Changes made in org.apache.commons.io.input.ByteBufferCleaner - Removed risk of ExceptionInInitializerError due to "sun.misc.Unsafe" not loadable when using the module path. - Introduced "jdk.internal.ref.Cleaner" as a JDK9+ alternative for "sun.misc.Cleaner". - If static member "CLEANER" cannot be initialized, an ExceptionInInitializerError can (theoretically) still occur, but should an ExceptionInInitializerError occur, it now provides the correct root cause whereas the previous implementation always provided the stacktrace of the Java 8 failure. Testing - No new test were introduced, all existing tests pass in Java 11 and Java 25. - I ran the tests with Java 8 as well, but got 2 failures out of 6621 tests. Both failures are in FileUtilsTest::testIsFileNewerOlder and don't seem to relate to my changes. It looks like a flaky test that has the potential to create different (timestamp) outcomes depending what combination of JVM version/vendor, operating system, file system and physical disks are used. - The surefire-plugin configuration was changed to allow module access to "sun.nio.ch" and "jdk.internal.ref" packages when running the build on Java 9+ using the already present build profiles. Runtime considerations when using Commons IO on the module path - By default the "jdk.internal.ref" and "sun.nio.ch" are packages are not accessible to the commons.io module. As such, the following needs to be added to the VM arguments to use Commons IO on the module path: "--add-opens=java.base/sun.nio.ch=org.apache.commons.io --add-opens=java.base/jdk.internal.ref=org.apache.commons.io" What could be better: - ByteBufferCleaner uses Class.forName to dynamically figure out if we are running on Java 8 or 9+. This could be solved more elegantly with a multi-release JAR with separate Java 8 and 9 versions of ByteBufferCleaner. The separate versions would not need Class.forName then. - Ideally the unit tests should have a full pass with the classes on the classpath and another full pass with the classes on the module path. The current build does classpath only. Why this should be merged: - Version 2.22.0 and the HEAD (f9de7a4) cannot use the ByteBufferCleaner with the Commons IO JAR on the module path. This PR fixes that. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
