Java 17 support #3815

sbesson · 2022-04-21T11:02:08Z

Java 17 LTS was released in September 2021 and in getting increasing used in the community by consumers of Bio-Formats e.g. bioformats2raw or QuPath. While there has been few reports so far indicating, this PR aims to bring full Java 17 support to Bio-Formats in the upcoming minor release, adding the required testing infrastructure and making the minimal set of code changes.

Summary of changes:

7ef4d15 and f6d9c4b add Java 17 to the GitHub Actions testing matrix
6bf30d7 and bf047fb make the minimal changes to the test-suite infrastructure to have the automated tests running against JDK 17
d27a825, 767733f, 43b572f, a315e5b and 7769fa8 contain the minimal reader changes required for the memoization on Java 17
07b8507 upgrades ome-codecs to 0.4.2 which includes the required upgrade of io.airlit.aircompressor to version 0.21. As demonstrated in the discussion of this PR, this change required Switch to Maven Artifact Resolver Ant tasks #3923 to fix the Ant build

Related work:

Make private Pattern field transient #3796 also fixes an issue exposed when using Java 17
BIOFORMATS-image: rotate between JDK versions devspace#194 proposes to add the infrastructure to alternate between JDK versions in the daily Bio-Formats CI coverage and BIOFORMATS-image: rotate between JDK 8, 11 and 17 devspace#196 adds JDK 17 to the rotation (currently used in `merge-ci.openmicroscopy.org)

dgault · 2022-05-23T14:39:10Z

The ome-poi bump to 5.3.5 has been removed as develop has now been bumped to 5.3.6

On a separate note, when running the test suite you will run into issues with the file leak detector using Java 17:
jenkinsci/lib-file-leak-detector#86

sbesson · 2022-05-23T16:37:21Z

On a separate note, when running the test suite you will run into issues with the file leak detector using Java 17:
jenkinsci/lib-file-leak-detector#86

Thanks for cross-linking to this issue which was definitely on my radar. The upgrade of file-leak-detector to 1.15 was necessary in order to get the test-suite simply running under Java 17. As per the nature of the changes, some of the logic might possibly have been disabled. On the other hand, file leak detection should at least remain functional both under Java 8 and Java 11 (as #3828 should demonstrate).

Also using the occasion to capture the progress on this investigation, with the set of changes above, the tests were executed once against the curated QA repository under Java 17 - see https://merge-ci.openmicroscopy.org/jenkins/job/BIOFORMATS-test-repo/1154 for the build logs. The run was overall successful and only failed on 6 folders. These will need closer examination and I expect changes similar to #3796 will be required.

sbesson · 2022-06-14T09:43:53Z

Testing the memo file generation on JDK 17 with one of the failing samples points at the serialization of loci.common.Location private field as a source of issue

failed to save memo file: /tmp/cache/uod/idr/repos/curated/ome-tiff/pam/SingleImage-09242010-1031-049/.SingleImage-09242010-1031-049.xml.bfmemo
com.esotericsoftware.kryo.KryoException: java.lang.IllegalArgumentException: Unable to create serializer "com.esotericsoftware.kryo.serializers.FieldSerializer" for class: java.io.File
Serialization trace:
file (loci.common.Location)
cfgFile (loci.formats.in.PrairieReader)
readers (loci.formats.ImageReader)
	at com.esotericsoftware.kryo.serializers.ObjectField.write(ObjectField.java:101)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.write(FieldSerializer.java:508)
	at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:575)
	at com.esotericsoftware.kryo.serializers.ObjectField.write(ObjectField.java:79)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.write(FieldSerializer.java:508)
	at com.esotericsoftware.kryo.Kryo.writeClassAndObject(Kryo.java:651)
	at com.esotericsoftware.kryo.serializers.DefaultArraySerializers$ObjectArraySerializer.write(DefaultArraySerializers.java:361)
	at com.esotericsoftware.kryo.serializers.DefaultArraySerializers$ObjectArraySerializer.write(DefaultArraySerializers.java:302)
	at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:575)
	at com.esotericsoftware.kryo.serializers.ObjectField.write(ObjectField.java:79)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.write(FieldSerializer.java:508)
	at com.esotericsoftware.kryo.Kryo.writeObject(Kryo.java:557)
	at loci.formats.Memoizer$KryoDeser.saveReader(Memoizer.java:206)
	at loci.formats.Memoizer.saveMemo(Memoizer.java:968)
	at loci.formats.Memoizer.setId(Memoizer.java:697)
	at loci.formats.ReaderWrapper.setId(ReaderWrapper.java:662)
	at loci.formats.tools.ImageInfo.testRead(ImageInfo.java:1035)
	at loci.formats.tools.ImageInfo.main(ImageInfo.java:1121)
Caused by: java.lang.IllegalArgumentException: Unable to create serializer "com.esotericsoftware.kryo.serializers.FieldSerializer" for class: java.io.File
	at com.esotericsoftware.kryo.factories.ReflectionSerializerFactory.makeSerializer(ReflectionSerializerFactory.java:65)
	at com.esotericsoftware.kryo.factories.ReflectionSerializerFactory.makeSerializer(ReflectionSerializerFactory.java:43)
	at com.esotericsoftware.kryo.Kryo.newDefaultSerializer(Kryo.java:396)
	at com.esotericsoftware.kryo.Kryo.getDefaultSerializer(Kryo.java:380)
	at com.esotericsoftware.kryo.util.DefaultClassResolver.registerImplicit(DefaultClassResolver.java:74)
	at com.esotericsoftware.kryo.Kryo.getRegistration(Kryo.java:508)
	at com.esotericsoftware.kryo.util.DefaultClassResolver.writeClass(DefaultClassResolver.java:97)
	at com.esotericsoftware.kryo.Kryo.writeClass(Kryo.java:540)
	at com.esotericsoftware.kryo.serializers.ObjectField.write(ObjectField.java:75)
	... 17 common frames omitted
Caused by: java.lang.reflect.InvocationTargetException: null
	at jdk.internal.reflect.GeneratedConstructorAccessor1.newInstance(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
	at java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
	at com.esotericsoftware.kryo.factories.ReflectionSerializerFactory.makeSerializer(ReflectionSerializerFactory.java:51)
	... 25 common frames omitted
Caused by: java.lang.reflect.InaccessibleObjectException: Unable to make field private final java.lang.String java.io.File.path accessible: module java.base does not "opens java.io" to unnamed module @6b06aa9b
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:354)
	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:297)
	at java.base/java.lang.reflect.Field.checkCanSetAccessible(Field.java:178)
	at java.base/java.lang.reflect.Field.setAccessible(Field.java:172)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.buildValidFields(FieldSerializer.java:283)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.rebuildCachedFields(FieldSerializer.java:216)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.rebuildCachedFields(FieldSerializer.java:157)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.<init>(FieldSerializer.java:150)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.<init>(FieldSerializer.java:134)
	at com.esotericsoftware.kryo.serializers.FieldSerializer.<init>(FieldSerializer.java:130)
	... 30 common frames omitted

From a quick review of https://merge-ci.openmicroscopy.org/jenkins/job/BIOFORMATS-test-repo/1154, several of the formats which failed the tests (cv7000, cellvoyager, prairie, volocity) include a Location private field

(base) sbesson@Sebastiens-MacBook-Pro bioformats % git grep "private.*Location .*;"
components/formats-bsd/src/loci/formats/Memoizer.java:  private Location realFile;
components/formats-bsd/src/loci/formats/tools/FakeImage.java:  private Location directoryRoot;
components/formats-gpl/src/loci/formats/in/CV7000Reader.java:  private Location parent;
components/formats-gpl/src/loci/formats/in/CellVoyagerReader.java:  private Location measurementFolder;
components/formats-gpl/src/loci/formats/in/CellVoyagerReader.java:  private Location imageFolder;
components/formats-gpl/src/loci/formats/in/CellVoyagerReader.java:  private Location measurementResultFile;
components/formats-gpl/src/loci/formats/in/CellVoyagerReader.java:  private Location omeMeasurementFile;
components/formats-gpl/src/loci/formats/in/PrairieReader.java:  private Location xmlFile, cfgFile, envFile;
components/formats-gpl/src/loci/formats/in/VolocityReader.java:  private Location dir = null;

Prevents memo saving issues with Java 17.

melissalinkert · 2022-06-14T21:19:48Z

EsotericSoftware/kryo#859 (comment) is maybe related. One approach is to switch the Location fields to just String. https://github.com/melissalinkert/bioformats/commits/java_17 has commits to do this, which seems to fix everything except ome-tiff and zeiss-czi.

sbesson · 2022-06-15T09:48:47Z

That's great. Do you want to add these commits to this branch so that we can get them testing in JDK 8/11 before re-enabling JDK17?

I think the errors in ome-tiff were caused by filesets read by PrairieReader so I am optimistic your changes will address these as well.

For zeiss-czi, a quick test shows the underlying exception is

Exception in thread "main" java.lang.ExceptionInInitializerError
	at io.airlift.compress.zstd.ZstdFrameDecompressor.verifyMagic(ZstdFrameDecompressor.java:952)
	at io.airlift.compress.zstd.ZstdFrameDecompressor.getDecompressedSize(ZstdFrameDecompressor.java:944)
	at io.airlift.compress.zstd.ZstdDecompressor.getDecompressedSize(ZstdDecompressor.java:96)
	at loci.formats.in.ZeissCZIReader$SubBlock.readPixelData(ZeissCZIReader.java:4101)
	at loci.formats.in.ZeissCZIReader$SubBlock.readPixelData(ZeissCZIReader.java:4017)
	at loci.formats.in.ZeissCZIReader$SubBlock.readPixelData(ZeissCZIReader.java:4012)
	at loci.formats.in.ZeissCZIReader.initFile(ZeissCZIReader.java:773)
	at loci.formats.FormatReader.setId(FormatReader.java:1473)
	at loci.formats.ImageReader.setId(ImageReader.java:861)
	at loci.formats.ReaderWrapper.setId(ReaderWrapper.java:662)
	at loci.formats.tools.ImageInfo.testRead(ImageInfo.java:1035)
	at loci.formats.tools.ImageInfo.main(ImageInfo.java:1121)
Caused by: io.airlift.compress.IncompatibleJvmException: Zstandard requires access to java.nio.Buffer raw address field
	at io.airlift.compress.zstd.UnsafeUtil.<clinit>(UnsafeUtil.java:53)
	... 12 more

which comes down to the io.airlift.aircompressor dependency.

This reverts commit 4e79c00.

sbesson · 2022-06-15T15:48:43Z

fb60d7c reverts the aircompressor dependency bump. While the new dependency compiled without issue with the Maven build system, this failed the Ant builds while retrieving dependencies

 [artifact:dependencies] Downloading: io/airlift/aircompressor/0.21/aircompressor-0.21.pom from repository central at https://repo.maven.apache.org/maven2
[artifact:dependencies] Transferring 6K from central
[artifact:dependencies] Downloading: io/airlift/airbase/112/airbase-112.pom from repository central at https://repo.maven.apache.org/maven2
[artifact:dependencies] Transferring 67K from central
[artifact:dependencies] Downloading: org/junit/junit-bom/5.8.0-M1/junit-bom-5.8.0-M1.pom from repository central at http://repo1.maven.org/maven2

Shortly, the issue is that the Ant build is using the maven-ant-tasks project to resolve dependencies. The goal of this change was to reduce the need for the dependencies to be declared twice and have both Ant and Maven using the POM file as the single source of truth. Unfortunately, this component is now retired, unsupported and internally uses the hardcoded HTTP version of the Central repository (which has now been updated to HTTPS - https://links.sonatype.com/central/501-https-required) to resolve parent POMs.

A couple of options from a quick brainstorming with @melissalinkert, rouhgly listed in order ranging from the minimal cost+value to the maximal cost+value:

edit the XML file and package a custom version of the JAR as mentioned in https://stackoverflow.com/questions/59948747/maven-ant-task-transitive-pom-dependencies-and-https-issue
migrate from maven-ant-tasks to the recommended maven artifact resolver alternative
remove the dual build system: drop Ant fully in favor of Maven or Gradle

Immediately, fb60d7c will allow us to test the other fixes and start considering options for the aircompressor dependency upgrade.

joshmoore · 2022-06-22T10:14:11Z

EsotericSoftware/kryo#859 (comment) is maybe related. One approach is to switch the Location fields to just String.

For for chiming in late, but just wanted to add that I assume since Location is in our code we could provide a serializer for it to not need to switch to Strings.

But in general, this incompatibility between Java internals and Kryo doesn't fill me with fuzzies.

imagesc-bot · 2022-09-15T22:56:23Z

This pull request has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/dicom-folder-to-ometiff/61402/13

jburel · 2022-10-03T14:46:36Z

components/formats-gpl/src/loci/formats/in/CellVoyagerReader.java


    /*
     * Open MeasurementSettings file
     */

    RandomAccessInputStream result =
-      new RandomAccessInputStream(measurementResultFile.getAbsolutePath());
+      new RandomAccessInputStream(measurementResultFile);


Maybe take advantage of try with resources here. This will be in the spirit of introducing new Java version

sbesson · 2022-10-31T09:14:28Z

After reincluding this in the daily CI builds, Saturday's run revealed only 2 formats are currently failing the tests on Java 17:

tiff and more specifically the zstd compressed samples
zeiss-czi - might be related to the above as this is the other format using zstd but the exception is eaten during the test creation step

See ome/bioformats#3815 (comment), airlift/aircompressor#125.

dgault · 2022-11-10T11:51:01Z

ome-codecs with the updated aircompressor has now been released and should be available to bump to https://github.com/ome/ome-codecs/releases/tag/v0.4.2

sbesson · 2022-11-10T13:32:07Z

Unfortunately upgrading ome-codecs to 0.4.2 brings us back to the build system issue previously raised in #3815 (comment). Unlike at the time of the previous comment, we now have a set of commits allowing to have all repository tests passing on JDK 8, 11 17. I'll evaluate the three options discussed previously and discuss them at an upcoming Formats meeting.

This reverts commit ba7e88b.

sbesson · 2023-01-12T16:45:02Z

With the inclusion of #3923, the Ant workflow can now be reactivated and Java 17 also added to the matrix. The CI builds have been passing consistently for the last few week with the repository tests being executed successfully on JDK 8, JDK 11, JDK 17. The latest state of this branch obviously needs to be re-evaluated but assuming no regression, there is nothing else coming from my side on this front.
The description has been updated to reflect more precisely the latest state of the proposed changes and these code changes are now ready for review and hopefully inclusion in the upcoming 6.12.0 release of Bio-Formats

dgault

All builds and tests have been stable with this PR included for a while now. Getting this merged for inclusion with the 6.12.0 release

sbesson added 2 commits April 21, 2022 09:16

Add Java 17 LTS to the GitHub CI matrix

7ef4d15

Mark inner static class as public

d27a825

sbesson mentioned this pull request Apr 21, 2022

Illegal reflective access by kryo #3659

Closed

This was referenced May 4, 2022

Fix the testing for JDK 8 /11 by using --build-arg ome/bio-formats-build#274

Merged

Fix compile issues with JDK 17 ome/ome-poi#9

Merged

Bump ome-poi version to 5.3.5

989cae9

sbesson changed the title ~~Java 17 GHA testing~~ Java 17 support May 11, 2022

sbesson added 2 commits May 18, 2022 11:19

Bump file-leak-detector to 1.15 including Java 17 fixes

6bf30d7

Try and declare Jenkins repository for consuming file-leak-detector:1.15

bf047fb

snoopycrimecop mentioned this pull request May 20, 2022

Logback: components bump #3827

Merged

Merge branch 'develop' into java_17

39b06cd

melissalinkert added 4 commits June 14, 2022 14:41

Prairie: switch Location fields to String

767733f

Prevents memo saving issues with Java 17.

CellVoyager: switch Location fields to String

43b572f

Prevents memo saving issues with Java 17.

CV7000: switch Location fields to String

a315e5b

Prevents memo saving issues with Java 17.

Volocity: switch Location fields to String

7769fa8

Prevents memo saving issues with Java 17.

Bump io.airlit.aircompressor to version 0.21

4e79c00

melissalinkert and others added 2 commits June 15, 2022 09:17

Merge branch 'java_17' into java_17

7e4ed22

Revert "Bump io.airlit.aircompressor to version 0.21"

fb60d7c

This reverts commit 4e79c00.

dgault added this to the 6.11.0 milestone Aug 9, 2022

sbesson mentioned this pull request Aug 29, 2022

BIOFORMATS-image: rotate between JDK versions ome/devspace#194

Merged

sbesson marked this pull request as draft October 3, 2022 13:36

jburel reviewed Oct 3, 2022

View reviewed changes

sbesson mentioned this pull request Oct 26, 2022

BIOFORMATS-image: rotate between JDK 8, 11 and 17 ome/devspace#196

Merged

sbesson added 2 commits October 26, 2022 17:09

Merge remote-tracking branch 'origin/develop' into java_17

c22f419

CellVoyager: use try-with-resources

26c056d

sbesson marked this pull request as ready for review October 26, 2022 18:18

melissalinkert added a commit to melissalinkert/ome-codecs that referenced this pull request Nov 1, 2022

Update aircompressor to 0.21

29cc85e

See ome/bioformats#3815 (comment), airlift/aircompressor#125.

melissalinkert mentioned this pull request Nov 1, 2022

Update aircompressor to 0.21 ome/ome-codecs#23

Merged

Bump ome-codecs to 0.4.2

07b8507

Temporarily disable Ant workflow

ba7e88b

snoopycrimecop mentioned this pull request Nov 22, 2022

Upload one set of artifacts from the GitHub Actions ant build #3912

Merged

sbesson mentioned this pull request Dec 9, 2022

Switch to Maven Artifact Resolver Ant tasks #3923

Merged

sbesson added 3 commits January 12, 2023 16:12

Revert "Temporarily disable Ant workflow"

c0aca82

This reverts commit ba7e88b.

Merge remote-tracking branch 'origin/develop' into java_17

9d90656

Add JDK 17 to the Ant testing matrix

f6d9c4b

sbesson requested review from dgault and melissalinkert January 12, 2023 16:45

melissalinkert approved these changes Jan 12, 2023

View reviewed changes

This was referenced Jan 19, 2023

upgrade setup-java action. Current version uses set-output #3933

Merged

Release artifacts #3934

Merged

dgault approved these changes Jan 31, 2023

View reviewed changes

dgault merged commit f0017bd into ome:develop Jan 31, 2023

sbesson deleted the java_17 branch January 31, 2023 15:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Java 17 support #3815

Java 17 support #3815

sbesson commented Apr 21, 2022 •

edited

Loading

dgault commented May 23, 2022

sbesson commented May 23, 2022 •

edited

Loading

sbesson commented Jun 14, 2022

melissalinkert commented Jun 14, 2022

sbesson commented Jun 15, 2022

sbesson commented Jun 15, 2022

joshmoore commented Jun 22, 2022

imagesc-bot commented Sep 15, 2022

jburel Oct 3, 2022

sbesson commented Oct 31, 2022

dgault commented Nov 10, 2022

sbesson commented Nov 10, 2022

sbesson commented Jan 12, 2023

dgault left a comment

Java 17 support #3815

Java 17 support #3815

Conversation

sbesson commented Apr 21, 2022 • edited Loading

dgault commented May 23, 2022

sbesson commented May 23, 2022 • edited Loading

sbesson commented Jun 14, 2022

melissalinkert commented Jun 14, 2022

sbesson commented Jun 15, 2022

sbesson commented Jun 15, 2022

joshmoore commented Jun 22, 2022

imagesc-bot commented Sep 15, 2022

jburel Oct 3, 2022

Choose a reason for hiding this comment

sbesson commented Oct 31, 2022

dgault commented Nov 10, 2022

sbesson commented Nov 10, 2022

sbesson commented Jan 12, 2023

dgault left a comment

Choose a reason for hiding this comment

sbesson commented Apr 21, 2022 •

edited

Loading

sbesson commented May 23, 2022 •

edited

Loading