Does this look like a race?

[Andrew Haley]

(Please forgive me if this should go to the AWT list or somesuch.  It
seems to me like this is not really a graphics-related issue, but one
of Java concurrency.)

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.  Do you agree?

Thanks,
Andrew.


    /* 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;
        }
    }