Skip to content

Commit

Permalink
6986863: ProfileDeferralMgr throwing ConcurrentModificationException
Browse files Browse the repository at this point in the history 
Reviewed-by: kizune
  • Loading branch information
@mrserb
mrserb committed Jan 29, 2021
1 parent ea2c447 commit 64a150c
Show file tree
Hide file tree
Showing 9 changed files with 255 additions and 337 deletions.
190 changes: 54 additions & 136 deletions src/java.desktop/share/classes/java/awt/color/ICC_Profile.java
View file
Expand Up @@ -55,10 +55,8 @@
import sun.java2d.cmm.CMSManager;
import sun.java2d.cmm.PCMM;
import sun.java2d.cmm.Profile;
import sun.java2d.cmm.ProfileActivator;
import sun.java2d.cmm.ProfileDataVerifier;
import sun.java2d.cmm.ProfileDeferralInfo;
import sun.java2d.cmm.ProfileDeferralMgr;

/**
* A representation of color profile data for device independent and device
Expand Down Expand Up @@ -93,10 +91,8 @@ public class ICC_Profile implements Serializable {
@Serial
private static final long serialVersionUID = -3938515861990936766L;

private transient Profile cmmProfile;

private transient ProfileDeferralInfo deferralInfo;
private transient ProfileActivator profileActivator;
private transient volatile Profile cmmProfile;
private transient volatile ProfileDeferralInfo deferralInfo;

// Registry of singleton profile objects for specific color spaces
// defined in the ColorSpace class (e.g. CS_sRGB), see
Expand Down Expand Up @@ -731,21 +727,15 @@ public class ICC_Profile implements Serializable {
* Constructs an {@code ICC_Profile} object with a given ID.
*/
ICC_Profile(Profile p) {
this.cmmProfile = p;
cmmProfile = p;
}

/**
* Constructs an {@code ICC_Profile} object whose loading will be deferred.
* The ID will be 0 until the profile is loaded.
*/
ICC_Profile(ProfileDeferralInfo pdi) {
this.deferralInfo = pdi;
this.profileActivator = new ProfileActivator() {
public void activate() throws ProfileDataException {
activateDeferredProfile();
}
};
ProfileDeferralMgr.registerDeferral(this.profileActivator);
deferralInfo = pdi;
}

/**
Expand Down Expand Up @@ -780,10 +770,6 @@ public static ICC_Profile getInstance(byte[] data) {

Profile p = null;

if (ProfileDeferralMgr.deferring) {
ProfileDeferralMgr.activateProfiles();
}

ProfileDataVerifier.verify(data);

try {
Expand Down Expand Up @@ -842,11 +828,11 @@ public static ICC_Profile getInstance (int cspace) {
* Enabling the appropriate access privileges is handled
* at a lower level.
*/
ProfileDeferralInfo pInfo =
ProfileDeferralInfo pdi =
new ProfileDeferralInfo("sRGB.pf",
ColorSpace.TYPE_RGB, 3,
CLASS_DISPLAY);
sRGBprofile = getDeferredInstance(pInfo);
sRGBprofile = new ICC_ProfileRGB(pdi);
}
thisProfile = sRGBprofile;
}
Expand All @@ -856,11 +842,11 @@ public static ICC_Profile getInstance (int cspace) {
case ColorSpace.CS_CIEXYZ:
synchronized(ICC_Profile.class) {
if (XYZprofile == null) {
ProfileDeferralInfo pInfo =
ProfileDeferralInfo pdi =
new ProfileDeferralInfo("CIEXYZ.pf",
ColorSpace.TYPE_XYZ, 3,
CLASS_ABSTRACT);
XYZprofile = getDeferredInstance(pInfo);
XYZprofile = new ICC_Profile(pdi);
}
thisProfile = XYZprofile;
}
Expand All @@ -870,11 +856,11 @@ public static ICC_Profile getInstance (int cspace) {
case ColorSpace.CS_PYCC:
synchronized(ICC_Profile.class) {
if (PYCCprofile == null) {
ProfileDeferralInfo pInfo =
ProfileDeferralInfo pdi =
new ProfileDeferralInfo("PYCC.pf",
ColorSpace.TYPE_3CLR, 3,
CLASS_COLORSPACECONVERSION);
PYCCprofile = getDeferredInstance(pInfo);
PYCCprofile = new ICC_Profile(pdi);
}
thisProfile = PYCCprofile;
}
Expand All @@ -884,11 +870,11 @@ public static ICC_Profile getInstance (int cspace) {
case ColorSpace.CS_GRAY:
synchronized(ICC_Profile.class) {
if (GRAYprofile == null) {
ProfileDeferralInfo pInfo =
ProfileDeferralInfo pdi =
new ProfileDeferralInfo("GRAY.pf",
ColorSpace.TYPE_GRAY, 1,
CLASS_DISPLAY);
GRAYprofile = getDeferredInstance(pInfo);
GRAYprofile = new ICC_ProfileGray(pdi);
}
thisProfile = GRAYprofile;
}
Expand All @@ -898,11 +884,11 @@ public static ICC_Profile getInstance (int cspace) {
case ColorSpace.CS_LINEAR_RGB:
synchronized(ICC_Profile.class) {
if (LINEAR_RGBprofile == null) {
ProfileDeferralInfo pInfo =
ProfileDeferralInfo pdi =
new ProfileDeferralInfo("LINEAR_RGB.pf",
ColorSpace.TYPE_RGB, 3,
CLASS_DISPLAY);
LINEAR_RGBprofile = getDeferredInstance(pInfo);
LINEAR_RGBprofile = new ICC_ProfileRGB(pdi);
}
thisProfile = LINEAR_RGBprofile;
}
Expand All @@ -916,26 +902,6 @@ public static ICC_Profile getInstance (int cspace) {
return thisProfile;
}

/**
* This method asserts system privileges, so is used only for the standard
* profiles.
*/
private static ICC_Profile getStandardProfile(final String name) {
return AccessController.doPrivileged(
new PrivilegedAction<ICC_Profile>() {
public ICC_Profile run() {
ICC_Profile p = null;
try {
p = getInstance(name);
} catch (IOException ex) {
throw new IllegalArgumentException(
"Can't load standard profile: " + name);
}
return p;
}
});
}

/**
* Constructs an {@code ICC_Profile} corresponding to the data in a file.
* {@code fileName} may be an absolute or a relative file specification.
Expand Down Expand Up @@ -997,13 +963,7 @@ public static ICC_Profile getInstance(String fileName) throws IOException {
* Profile data
*/
public static ICC_Profile getInstance(InputStream s) throws IOException {
byte[] profileData;

if (s instanceof ProfileDeferralInfo) {
/* hack to detect profiles whose loading can be deferred */
return getDeferredInstance((ProfileDeferralInfo) s);
}

byte[] profileData;
if ((profileData = getProfileDataFromStream(s)) == null) {
throw new IllegalArgumentException("Invalid ICC Profile Data");
}
Expand Down Expand Up @@ -1035,61 +995,32 @@ static byte[] getProfileDataFromStream(InputStream s) throws IOException {
}

/**
* Constructs an {@code ICC_Profile} for which the actual loading of the
* profile data from a file and the initialization of the CMM should be
* deferred as long as possible. Deferral is only used for standard
* profiles. If deferring is disabled, then getStandardProfile() ensures
* that all of the appropriate access privileges are granted when loading
* this profile. If deferring is enabled, then the deferred activation code
* will take care of access privileges.
*
* @see #activateDeferredProfile()
* Activates the deferred standard profiles. Implementation of this method
* mimics the old behaviour when the CMMException and IOException were
* wrapped by the ProfileDataException, and the ProfileDataException itself
* was ignored during activation.
*/
static ICC_Profile getDeferredInstance(ProfileDeferralInfo pdi) {
if (!ProfileDeferralMgr.deferring) {
return getStandardProfile(pdi.filename);
}
if (pdi.colorSpaceType == ColorSpace.TYPE_RGB) {
return new ICC_ProfileRGB(pdi);
} else if (pdi.colorSpaceType == ColorSpace.TYPE_GRAY) {
return new ICC_ProfileGray(pdi);
} else {
return new ICC_Profile(pdi);
}
}


void activateDeferredProfile() throws ProfileDataException {
byte[] profileData;
final String fileName = deferralInfo.filename;

profileActivator = null;
deferralInfo = null;
InputStream is = getStandardProfileInputStream(fileName);
if (is == null) {
throw new ProfileDataException("Cannot open file " + fileName);
}
try {
profileData = getProfileDataFromStream(is);
is.close(); /* close the file */
}
catch (IOException e) {
ProfileDataException pde = new
ProfileDataException("Invalid ICC Profile Data" + fileName);
pde.initCause(e);
throw pde;
}
if (profileData == null) {
throw new ProfileDataException("Invalid ICC Profile Data" +
fileName);
}
try {
cmmProfile = CMSManager.getModule().loadProfile(profileData);
} catch (CMMException c) {
ProfileDataException pde = new
ProfileDataException("Invalid ICC Profile Data" + fileName);
pde.initCause(c);
throw pde;
private void activate() {
if (cmmProfile == null) {
synchronized (this) {
if (cmmProfile != null) {
return;
}
var is = getStandardProfileInputStream(deferralInfo.filename);
if (is == null) {
return;
}
try {
byte[] data = getProfileDataFromStream(is);
if (data != null) {
cmmProfile = CMSManager.getModule().loadProfile(data);
// from now we cannot use the deferred value, drop it
deferralInfo = null;
}
is.close(); /* close the stream */
} catch (CMMException | IOException ignore) {
}
}
}
}

Expand Down Expand Up @@ -1130,11 +1061,9 @@ public int getProfileClass() {
byte[] theHeader;
int theClassSig, theClass;

if (deferralInfo != null) {
return deferralInfo.profileClass; /* Need to have this info for
ICC_ColorSpace without
causing a deferred profile
to be loaded */
ProfileDeferralInfo info = deferralInfo;
if (info != null) {
return info.profileClass;
}

theHeader = getData(icSigHead);
Expand Down Expand Up @@ -1190,12 +1119,11 @@ public int getProfileClass() {
* {@code ColorSpace} class
*/
public int getColorSpaceType() {
if (deferralInfo != null) {
return deferralInfo.colorSpaceType; /* Need to have this info for
ICC_ColorSpace without
causing a deferred profile
to be loaded */
ProfileDeferralInfo info = deferralInfo;
if (info != null) {
return info.colorSpaceType;
}
activate();
return getColorSpaceType(cmmProfile);
}

Expand Down Expand Up @@ -1223,9 +1151,7 @@ static int getColorSpaceType(Profile p) {
* {@code ColorSpace} class
*/
public int getPCSType() {
if (ProfileDeferralMgr.deferring) {
ProfileDeferralMgr.activateProfiles();
}
activate();
return getPCSType(cmmProfile);
}

Expand Down Expand Up @@ -1283,9 +1209,7 @@ public byte[] getData() {
int profileSize;
byte[] profileData;

if (ProfileDeferralMgr.deferring) {
ProfileDeferralMgr.activateProfiles();
}
activate();

PCMM mdl = CMSManager.getModule();

Expand Down Expand Up @@ -1315,9 +1239,7 @@ public byte[] getData() {
*/
public byte[] getData(int tagSignature) {

if (ProfileDeferralMgr.deferring) {
ProfileDeferralMgr.activateProfiles();
}
activate();

return getData(cmmProfile, tagSignature);
}
Expand Down Expand Up @@ -1363,9 +1285,7 @@ static byte[] getData(Profile p, int tagSignature) {
*/
public void setData(int tagSignature, byte[] tagData) {

if (ProfileDeferralMgr.deferring) {
ProfileDeferralMgr.activateProfiles();
}
activate();

CMSManager.getModule().setTagData(cmmProfile, tagSignature, tagData);
}
Expand Down Expand Up @@ -1417,11 +1337,9 @@ public int getNumComponents() {
byte[] theHeader;
int theColorSpaceSig, theNumComponents;

if (deferralInfo != null) {
return deferralInfo.numComponents; /* Need to have this info for
ICC_ColorSpace without
causing a deferred profile
to be loaded */
ProfileDeferralInfo info = deferralInfo;
if (info != null) {
return info.numComponents;
}
theHeader = getData(icSigHead);

Expand Down
25 changes: 10 additions & 15 deletions src/java.desktop/share/classes/java/awt/image/ColorConvertOp.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2017, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -36,16 +36,18 @@

package java.awt.image;

import java.awt.Point;
import java.awt.Graphics2D;
import java.awt.color.*;
import sun.java2d.cmm.ColorTransform;
import java.awt.Point;
import java.awt.RenderingHints;
import java.awt.color.ColorSpace;
import java.awt.color.ICC_ColorSpace;
import java.awt.color.ICC_Profile;
import java.awt.geom.Point2D;
import java.awt.geom.Rectangle2D;

import sun.java2d.cmm.CMSManager;
import sun.java2d.cmm.ProfileDeferralMgr;
import sun.java2d.cmm.ColorTransform;
import sun.java2d.cmm.PCMM;
import java.awt.geom.Rectangle2D;
import java.awt.geom.Point2D;
import java.awt.RenderingHints;

/**
* This class performs a pixel-by-pixel color conversion of the data in
Expand Down Expand Up @@ -77,13 +79,6 @@ public class ColorConvertOp implements BufferedImageOp, RasterOp {
boolean gotProfiles;
float[] srcMinVals, srcMaxVals, dstMinVals, dstMaxVals;

/* the class initializer */
static {
if (ProfileDeferralMgr.deferring) {
ProfileDeferralMgr.activateProfiles();
}
}

/**
* Constructs a new ColorConvertOp which will convert
* from a source color space to a destination color space.
Expand Down

8 comments on commit 64a150c

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

@gnu-andrew
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 64a150c Dec 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnu-andrew Could not automatically backport 64a150c5 to openjdk/jdk11u-dev due to conflicts in the following files:

  • src/java.desktop/share/classes/java/awt/color/ICC_Profile.java
  • src/java.desktop/share/classes/sun/java2d/cmm/ProfileDeferralMgr.java

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk11u-dev:

$ git checkout -b gnu-andrew-backport-64a150c5
$ git fetch --no-tags https://git.openjdk.java.net/jdk 64a150c518ed1a936cd6e4cb5e60060243496901
$ git cherry-pick --no-commit 64a150c518ed1a936cd6e4cb5e60060243496901
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 64a150c518ed1a936cd6e4cb5e60060243496901'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk11u-dev with the title Backport 64a150c518ed1a936cd6e4cb5e60060243496901.

@mrserb
Copy link
Member Author

@mrserb mrserb commented on 64a150c Dec 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gnu-andrew I can take care about this backport and the follow up fixes.

@gnu-andrew
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What follow-up fixes? I don't see any linked from the bug.

@mrserb
Copy link
Member Author

@mrserb mrserb commented on 64a150c Dec 20, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example this one JDK-8260695 and others. When the current bug was fixed the follow-up code start to fail(that issues were hidden by this one).

@mrserb
Copy link
Member Author

@mrserb mrserb commented on 64a150c Jan 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk11u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on 64a150c Jan 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrserb Could not automatically backport 64a150c5 to openjdk/jdk11u-dev due to conflicts in the following files:

  • src/java.desktop/share/classes/java/awt/color/ICC_Profile.java
  • src/java.desktop/share/classes/sun/java2d/cmm/ProfileDeferralMgr.java

To manually resolve these conflicts run the following commands in your personal fork of openjdk/jdk11u-dev:

$ git checkout -b mrserb-backport-64a150c5
$ git fetch --no-tags https://git.openjdk.java.net/jdk 64a150c518ed1a936cd6e4cb5e60060243496901
$ git cherry-pick --no-commit 64a150c518ed1a936cd6e4cb5e60060243496901
$ # Resolve conflicts
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport 64a150c518ed1a936cd6e4cb5e60060243496901'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk11u-dev with the title Backport 64a150c518ed1a936cd6e4cb5e60060243496901.

Please sign in to comment.