Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8260695: The java.awt.color.ICC_Profile#getData/getData(int) are not thread safe #2330

Closed
wants to merge 8 commits into from
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -1186,22 +1186,8 @@ public void write(OutputStream s) throws IOException {
* @see #setData(int, byte[])
*/
public byte[] getData() {
int profileSize;
byte[] profileData;

activate();

PCMM mdl = CMSManager.getModule();

/* get the number of bytes needed for this profile */
profileSize = mdl.getProfileSize(cmmProfile);

profileData = new byte [profileSize];

/* get the data for the profile */
mdl.getProfileData(cmmProfile, profileData);

return profileData;
return CMSManager.getModule().getProfileData(cmmProfile);

This comment has been minimized.

@mrserb

mrserb Feb 1, 2021
Author Member

Just one step, the array will be created in the native code under proper synchronization.

}

/**
@@ -1225,25 +1211,12 @@ public void write(OutputStream s) throws IOException {
}


static byte[] getData(Profile p, int tagSignature) {
int tagSize;
byte[] tagData;

private static byte[] getData(Profile p, int tagSignature) {
try {
PCMM mdl = CMSManager.getModule();

/* get the number of bytes needed for this tag */
tagSize = mdl.getTagSize(p, tagSignature);

tagData = new byte[tagSize]; /* get an array for the tag */

/* get the tag's data */
mdl.getTagData(p, tagSignature, tagData);
} catch(CMMException c) {
tagData = null;
return CMSManager.getModule().getTagData(p, tagSignature);

This comment has been minimized.

@mrserb

mrserb Feb 1, 2021
Author Member

Just one step, the array will be created in the native code under proper synchronization.

} catch (CMMException c) {
return null;
}

return tagData;
}

/**
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2006, 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
@@ -100,33 +100,19 @@ public Profile loadProfile(byte[] data) {
return p;
}

public int getProfileSize(Profile p) {

This comment has been minimized.

@azvegint

azvegint Feb 7, 2021
Member

Not sure if it is a big loss, but it looks like we are no longer printing profile and tag sizes under CMMTracer after the fix.

System.err.print(cName + ".getProfileSize(ID=" + p + ")");
int size = tcmm.getProfileSize(p);
System.err.println("=" + size);
return size;
}

public void getProfileData(Profile p, byte[] data) {
public byte[] getProfileData(Profile p) {
System.err.print(cName + ".getProfileData(ID=" + p + ") ");
byte[] data = tcmm.getProfileData(p);
System.err.println("requested " + data.length + " byte(s)");
tcmm.getProfileData(p, data);
}

public int getTagSize(Profile p, int tagSignature) {
System.err.printf(cName + ".getTagSize(ID=%x, TagSig=%s)",
p, signatureToString(tagSignature));
int size = tcmm.getTagSize(p, tagSignature);
System.err.println("=" + size);
return size;
return data;
}

public void getTagData(Profile p, int tagSignature,
byte[] data) {
public byte[] getTagData(Profile p, int tagSignature) {
System.err.printf(cName + ".getTagData(ID=%x, TagSig=%s)",
p, signatureToString(tagSignature));
byte[] data = tcmm.getTagData(p, tagSignature);
System.err.println(" requested " + data.length + " byte(s)");
tcmm.getTagData(p, tagSignature, data);
return data;
}

public void setTagData(Profile p, int tagSignature,
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2006, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2006, 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
@@ -33,10 +33,8 @@

/* methods invoked from ICC_Profile */
public Profile loadProfile(byte[] data);
public int getProfileSize(Profile p);
public void getProfileData(Profile p, byte[] data);
public void getTagData(Profile p, int tagSignature, byte[] data);
public int getTagSize(Profile p, int tagSignature);
public byte[] getProfileData(Profile p);
public byte[] getTagData(Profile p, int tagSignature);
public void setTagData(Profile p, int tagSignature, byte[] data);

/* methods for creating ColorTransforms */
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2007, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2007, 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
@@ -58,45 +58,23 @@ private LCMSProfile getLcmsProfile(Profile p) {
}

@Override
public int getProfileSize(final Profile p) {
synchronized (p) {
return getProfileSizeNative(getLcmsProfile(p).getLcmsPtr());
public byte[] getProfileData(final Profile p) {
LCMSProfile lcmsProfile = getLcmsProfile(p);
synchronized (lcmsProfile) {
return getProfileDataNative(lcmsProfile.getLcmsPtr());
}
}

private native int getProfileSizeNative(long ptr);

@Override
public void getProfileData(final Profile p, byte[] data) {
synchronized (p) {
getProfileDataNative(getLcmsProfile(p).getLcmsPtr(), data);
}
}

private native void getProfileDataNative(long ptr, byte[] data);

@Override
public int getTagSize(Profile p, int tagSignature) {
final LCMSProfile profile = getLcmsProfile(p);

synchronized (profile) {
TagData t = profile.getTag(tagSignature);
return t == null ? 0 : t.getSize();
}
}
private native byte[] getProfileDataNative(long ptr);

static native byte[] getTagNative(long profileID, int signature);

@Override
public void getTagData(Profile p, int tagSignature, byte[] data)
{
final LCMSProfile profile = getLcmsProfile(p);

synchronized (profile) {
TagData t = profile.getTag(tagSignature);
if (t != null) {
t.copyDataTo(data);
}
public byte[] getTagData(Profile p, int tagSignature) {
final LCMSProfile lcmsProfile = getLcmsProfile(p);
synchronized (lcmsProfile) {
TagData t = lcmsProfile.getTag(tagSignature);
return t != null ? t.getData() : null;
}
}

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2013, 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
@@ -25,9 +25,8 @@

package sun.java2d.cmm.lcms;

import java.awt.color.CMMException;
import java.util.Arrays;
import java.util.HashMap;

import sun.java2d.cmm.Profile;

final class LCMSProfile extends Profile {
@@ -55,55 +54,40 @@ void clearTagCache() {
tagCache.clear();
}

static class TagCache {
final LCMSProfile profile;
private HashMap<Integer, TagData> tags;
private static final class TagCache {
private final LCMSProfile profile;
private final HashMap<Integer, TagData> tags = new HashMap<>();

TagCache(LCMSProfile p) {
private TagCache(LCMSProfile p) {
profile = p;
tags = new HashMap<>();
}

TagData getTag(int sig) {
private TagData getTag(int sig) {
TagData t = tags.get(sig);
if (t == null) {
byte[] tagData = LCMS.getTagNative(profile.getNativePtr(), sig);
if (tagData != null) {
t = new TagData(sig, tagData);
t = new TagData(tagData);
tags.put(sig, t);
}
}
return t;
}

void clear() {
private void clear() {
tags.clear();
}
}

static class TagData {
private int signature;
private byte[] data;
static final class TagData {
private final byte[] data;

TagData(int sig, byte[] data) {
this.signature = sig;
TagData(byte[] data) {
this.data = data;
}

int getSize() {
return data.length;
}

byte[] getData() {
return Arrays.copyOf(data, data.length);
}

void copyDataTo(byte[] dst) {
System.arraycopy(data, 0, dst, 0, data.length);
}

int getSignature() {
return signature;
return data.clone();
}
}
}
@@ -309,70 +309,46 @@ JNIEXPORT jlong JNICALL Java_sun_java2d_cmm_lcms_LCMS_loadProfileNative
return ptr_to_jlong(sProf);
}

/*
* Class: sun_java2d_cmm_lcms_LCMS
* Method: getProfileSizeNative
* Signature: (J)I
*/
JNIEXPORT jint JNICALL Java_sun_java2d_cmm_lcms_LCMS_getProfileSizeNative
(JNIEnv *env, jobject obj, jlong id)
{
lcmsProfile_p sProf = (lcmsProfile_p)jlong_to_ptr(id);
cmsUInt32Number pfSize = 0;

if (cmsSaveProfileToMem(sProf->pf, NULL, &pfSize) && ((jint)pfSize > 0)) {
return (jint)pfSize;
} else {
JNU_ThrowByName(env, "java/awt/color/CMMException",
"Can not access specified profile.");
return -1;
}
}

/*
* Class: sun_java2d_cmm_lcms_LCMS
* Method: getProfileDataNative
* Signature: (J[B)V
*/
JNIEXPORT void JNICALL Java_sun_java2d_cmm_lcms_LCMS_getProfileDataNative
(JNIEnv *env, jobject obj, jlong id, jbyteArray data)
JNIEXPORT jbyteArray JNICALL Java_sun_java2d_cmm_lcms_LCMS_getProfileDataNative
(JNIEnv *env, jobject obj, jlong id)
{
lcmsProfile_p sProf = (lcmsProfile_p)jlong_to_ptr(id);
jint size;
jbyte* dataArray;
cmsUInt32Number pfSize = 0;
cmsBool status;

// determine actual profile size
if (!cmsSaveProfileToMem(sProf->pf, NULL, &pfSize)) {
JNU_ThrowByName(env, "java/awt/color/CMMException",
"Can not access specified profile.");
return;
return NULL;
}

// verify java buffer capacity
size = (*env)->GetArrayLength(env, data);
if (0 >= size || pfSize > (cmsUInt32Number)size) {
JNU_ThrowByName(env, "java/awt/color/CMMException",
"Insufficient buffer capacity.");
return;
jbyteArray data = (*env)->NewByteArray(env, pfSize);

This comment has been minimized.

@mrserb

mrserb Feb 1, 2021
Author Member

Here we create the new array.

if (data == NULL) {
// An exception should have already been thrown.
return NULL;
}

dataArray = (*env)->GetByteArrayElements (env, data, 0);
jbyte* dataArray = (*env)->GetByteArrayElements(env, data, 0);
if (dataArray == NULL) {
// An exception should have already been thrown.
return;
return NULL;
}

status = cmsSaveProfileToMem(sProf->pf, dataArray, &pfSize);
cmsBool status = cmsSaveProfileToMem(sProf->pf, dataArray, &pfSize);

(*env)->ReleaseByteArrayElements (env, data, dataArray, 0);
(*env)->ReleaseByteArrayElements(env, data, dataArray, 0);

if (!status) {
JNU_ThrowByName(env, "java/awt/color/CMMException",
"Can not access specified profile.");
return;
return NULL;
}
return data;
}

/* Get profile header info */