-
Bug
-
Resolution: Fixed
-
P3
-
None
-
None
-
b130
-
generic
-
generic
This is in JDK9, sun.font.GlyphList. There is a non-volatile boolean
inUse, and it is not always written in a synchronized block. It is
used to allow exclusive access to a singleton instance.
It seems to me that, at a minimum, inUse should be volatile, or e.g.
strikelist might be overwritten to null after some other thread has
started using this GlyphList.
/* This scheme creates a singleton GlyphList which is checked out
* for use. Callers who find its checked out create one that after use
* is discarded. This means that in a MT-rendering environment,
* there's no need to synchronise except for that one instance.
* Fewer threads will then need to synchronise, perhaps helping
* throughput on a MP system. If for some reason the reusable
* GlyphList is checked out for a long time (or never returned?) then
* we would end up always creating new ones. That situation should not
* occur and if it did, it would just lead to some extra garbage being
* created.
private static GlyphList reusableGL = new GlyphList();
private static boolean inUse;
...
public static GlyphList getInstance() {
/* The following heuristic is that if the reusable instance is
* in use, it probably still will be in a micro-second, so avoid
* synchronising on the class and just allocate a new instance.
* The cost is one extra boolean test for the normal case, and some
* small number of cases where we allocate an extra object when
* in fact the reusable one would be freed very soon.
*/
if (inUse) {
return new GlyphList();
} else {
synchronized(GlyphList.class) {
if (inUse) {
return new GlyphList();
} else {
inUse = true;
return reusableGL;
}
}
}
}
...
/* There's a reference equality test overhead here, but it allows us
* to avoid synchronizing for GL's that will just be GC'd. This
* helps MP throughput.
*/
public void dispose() {
if (this == reusableGL) {
if (graybits != null && graybits.length > MAXGRAYLENGTH) {
graybits = null;
}
usePositions = false;
strikelist = null; // remove reference to the strike list
inUse = false;
}
}
inUse, and it is not always written in a synchronized block. It is
used to allow exclusive access to a singleton instance.
It seems to me that, at a minimum, inUse should be volatile, or e.g.
strikelist might be overwritten to null after some other thread has
started using this GlyphList.
/* This scheme creates a singleton GlyphList which is checked out
* for use. Callers who find its checked out create one that after use
* is discarded. This means that in a MT-rendering environment,
* there's no need to synchronise except for that one instance.
* Fewer threads will then need to synchronise, perhaps helping
* throughput on a MP system. If for some reason the reusable
* GlyphList is checked out for a long time (or never returned?) then
* we would end up always creating new ones. That situation should not
* occur and if it did, it would just lead to some extra garbage being
* created.
private static GlyphList reusableGL = new GlyphList();
private static boolean inUse;
...
public static GlyphList getInstance() {
/* The following heuristic is that if the reusable instance is
* in use, it probably still will be in a micro-second, so avoid
* synchronising on the class and just allocate a new instance.
* The cost is one extra boolean test for the normal case, and some
* small number of cases where we allocate an extra object when
* in fact the reusable one would be freed very soon.
*/
if (inUse) {
return new GlyphList();
} else {
synchronized(GlyphList.class) {
if (inUse) {
return new GlyphList();
} else {
inUse = true;
return reusableGL;
}
}
}
}
...
/* There's a reference equality test overhead here, but it allows us
* to avoid synchronizing for GL's that will just be GC'd. This
* helps MP throughput.
*/
public void dispose() {
if (this == reusableGL) {
if (graybits != null && graybits.length > MAXGRAYLENGTH) {
graybits = null;
}
usePositions = false;
strikelist = null; // remove reference to the strike list
inUse = false;
}
}