NPE in ICC_Profile.getInstance()

[tvasenin]

On JDK11, we've started to use TwelveMonkeys recently and hit the following reproducible NPE in out tests:

 java.lang.NullPointerException: null
   at java.desktop/java.awt.color.ICC_Profile.activateDeferredProfile(ICC_Profile.java:1081)
   at java.desktop/java.awt.color.ICC_Profile$1.activate(ICC_Profile.java:746)
   at java.desktop/sun.java2d.cmm.ProfileDeferralMgr.activateProfiles(ProfileDeferralMgr.java:95)
   at java.desktop/java.awt.color.ICC_Profile.getInstance(ICC_Profile.java:784)
   at java.desktop/java.awt.color.ICC_Profile.getInstance(ICC_Profile.java:1026)
   at com.twelvemonkeys.imageio.plugins.tiff.TIFFImageReader.getICCProfile(TIFFImageReader.java:2495)
   at com.twelvemonkeys.imageio.plugins.tiff.TIFFImageReader.getRawImageType(TIFFImageReader.java:468)
   at com.twelvemonkeys.imageio.plugins.tiff.TIFFImageReader.getImageTypes(TIFFImageReader.java:879)
   at com.twelvemonkeys.imageio.plugins.tiff.TIFFImageReader.read(TIFFImageReader.java:908)
   at java.desktop/javax.imageio.ImageReader.readAsRenderedImage(ImageReader.java:1573)

The root cause (basically) is thread-unsafe ProfileDeferralMgr:
https://bugs.openjdk.java.net/browse/JDK-8058973

The bug was fixed in JDK 17+:
openjdk/jdk@64a150c5

Since this fix is unlikely to be backported to earlier JDKs, the ask here is to implement the workaround in TwelveMonkeys.

The proposal is to add this static initializer to all classes where ICC_Profile.getInstance() is called:

// Workaround for https://bugs.openjdk.java.net/browse/JDK-8058973
// Does nothing useful on Java 17+
static {
    // Trigger ColorConvertOp static initializer that calls ProfileDeferralMgr.activateProfiles()
    new ColorConvertOp(null);
}

It might be a good idea to consolidate ICC_Profile.getInstance() calls to minimize the number of the affected classes.

[haraldk]

Thanks Timofey,

I'm aware of the JDK bug as it has troubled me and users of the library for years. I was hoping for the JDK fix to be backported to at least JDK 11, but I guess we weren't that lucky...

Anyway, I think your workaround is interesting, as it explicitly (well, as close as it gets) invokes activateProfiles(), without too much fuss (I've usually recommended using ICC_Profile.getInstance(CS_sRGB).getData() to force activation).

I believe even Class.forName("java.awt.image.ColorConvertOp") would do, as we only need the static initializer to run.

Now, most of the ICC profile handling in the library is already consolidated in the com.twelvemonkeys.imageio.color.ColorSpaces class and there is a hack in place (using profile.getData() as mentioned above) that forces profile activation in a static initializer already. The problem with this approach is that there is no way our library can force other application/3rd party code to initialize this class before accessing ICC profiles. It's perfectly possible that the profile deferral mechanism is in a bad state already, when we hit that block. So, basically it doesn't work as intended.

I've been thinking about this problem for a while, and I think we could do slightly better, using a hack that exploits the SPI mechanism to activate profiles before any attempt at using ImageIO plugins. It's still not 100% bullet proof, as applications could still use ICC profiles before ImageIO SPIs are loaded, or our plugins could be loaded "lazily" (ie. ImageIO.scanForPlugins()), like in a web application. But perhaps better than what we have today...

I still think the only 100% sure way is to include your own startup code, that forces profile activation.

Let me know what you think, or if you have ideas on how we can make sure the code is actually run "early enough", make sure to share them! 😀

--
Harald K

[tvasenin]

Searching for ICC_Profile.getInstance finds 7 non-test classes in the repo 😃 (one of them is TIFFImageReader, from where it's called in my stack trace)

I think moving all ICC_Profile.getInstance() calls into ColorSpaces class (and optionally updating the static initializer to the new proposal) should be good enough -- at least this should guarantee the library itself won't cause the issue anymore.

As for moving the initialization to SPI, I don't think it's worth doing it right now.

Also, it would be a good idea to mention this workaround somewhere in README / documentation (or maybe in exception message text 😎) -- that'll help people, who hit this error again for any reason, to correctly report it or add the workaround in their startup code.

[tvasenin]

I believe even Class.forName("java.awt.image.ColorConvertOp") would do, as we only need the static initializer to run.

Yes, it should also initialize the class -- even better approach than creating a new instance.

[haraldk]

Searching for ICC_Profile.getInstance finds 7 non-test classes in the repo 😃 (one of them is TIFFImageReader, from where it's called in my stack trace)

I think moving all ICC_Profile.getInstance() calls into ColorSpaces class (and optionally updating the static initializer to the new proposal) should be good enough -- at least this should guarantee the library itself won't cause the issue anymore.

Excellent! You are halfway to making the pull request already! 😀

Best regards,

--
Harald K

[mrserb]

Probably it will be good to prepare the reasoning why these bugs are causing so much troubles for so many users and it would be nice to back port the corresponding fixes from JDK17 to JDK 11?
You can file such request here: https://github.com/corretto/corretto-11/issues

[haraldk]

@mrserb I completely agree. But I don't think this is specific to Corretto, so I guess it should be filed at OpenJDK https://bugs.openjdk.java.net/.

PS: Anyone can do this, it doesn't have to be me. 😀

[haraldk]

New and improved version of profile activation workaround will be out in 3.8.0.
Parts of it will be included in 3.7.1.

Still, it will not fix the underlying problem, and upgrading to JDK 17 or other version that has the JDK 17 fix backported is highly recommended.