On Mon, 11 Jul 2022 10:01:18 GMT, Maxim Kartashev <[email protected]>
wrote:
> Correct. Now let's suppose that the rendering thread - unblocked at the time
> - is adding more records such that
> ArrayList needs to grow. Now ArrayList starts relocating its underlying
> storage array at the time as dispose() returns
First, the only way that list grows is here :
share/classes/sun/font/StrikeCache.java
static void disposeStrike(final FontStrikeDisposer disposer) {
..
rq.flushAndInvokeNow(new Runnable() {
public void run() {
doDispose(disposer);
Disposer.pollRemove();
}
});
..
}
where Disposer.pollRemove() is adding to the ArrayList deferredRecords
This disposeStrike method() is called from the dispose() method in
share/classes/sun/font/FontStrikeDisposer.java
So the ArrayList must have FINISHED "relocating its underlying storage array"
BEFORE dispose() returns.
So all the following text you have implying concurrent adds / removes etc etc
doesn't make sense to me.
What I can imagine is that the storage change may not yet be visible to the
Disposer thread,
or becomes visible as it is in the process of clearing the deferred records.
So the change to use the ConcurrentLinkedDeque would - if it does what I
suppose - fix that.
it would ensure all the changes made on the render thread are immediately
visible to the dispose thread.
So let's take that change since it can only make things safer
However I don't think you've explained how it leads to the crash.
Your comment " "double-free" is likely with the second free called from one of
the dispose() methods"
and the surrounding text again doesn't add up to me. I read you as saying there
are two dispose() methods
but there shouldn't be two operating on the same data and the Strike one is
synchronized and ensures
it only frees once ..
My guess - and the stack trace in the bug perhaps bears this out since it
implicates BufImgSurfaceData
- nothing to do with fonts .. is that this is a non-synchronized dispose()
method
allocated from here
0x0000000107afc4e0(0x00007ff636c574a0)
java.lang.Exception
at
java.desktop/sun.java2d.DefaultDisposerRecord.<init>(DefaultDisposerRecord.java:55)
at java.desktop/sun.java2d.Disposer.addRecord(Disposer.java:107)
at java.desktop/sun.awt.image.BufImgSurfaceData.initRaster(Native Method)
And DrawRotatedStringUsingRotatedFont.java does cycle through a lot of
BufferedImage instances too ..
> Are you implying that
> test/jdk/java/awt/Graphics2D/DrawString/DisposerTest.java is useless in the
> headless mode?
> Then I can mark it as headful.
It is useless to test this in headless mode since there is no RenderQueue ..
and so this whole scenario never happens
I think it sufficient to make the EXISTING test headful .. the harness will
report a failure automatically if there's a crash
Both new tests don't seem needed and the 2nd one doesn't seem to be testing the
issue at all ..
as I wrote above we do not run these concurrently and your test does nothing to
block the disposer thread.
-------------
PR: https://git.openjdk.org/jdk/pull/9362
