RFR: 8289208: Test DrawRotatedStringUsingRotatedFont.java occasionally crashes on MacOS

Maxim Kartashev mkartashev at openjdk.org
Mon Aug 1 14:23:56 UTC 2022


On Mon, 4 Jul 2022 09:31:05 GMT, Maxim Kartashev <mkartashev at openjdk.org> wrote:

> Java2D's `Disposer` maintains a record of objects to dispose of with the help of a collection that isn't thread safe. When `DisposerRecords` objects are being added to it at the same time as others are being disposed on the Toolkit thread, chaos ensues.
> 
> This commit replaces the collection with a thread-safe  one, more consistently guards against exceptions in individual disposers, and adds exception's stacktraces printing in order to facilitate said exceptions' debugging, which are otherwise hard to pinpoint without code modification. 
> 
> Originally, the bug was caught on MacOS running an existing test (`DrawRotatedStringUsingRotatedFont`) that would occasionally crash the VM (probably due to double-free detected by libc that abort()'s in this case). It may take many re-tries to reproduce and this wasn't  observed on Linux. The new test (`test/jdk/sun/java2d/Disposer/TestDisposerRace.java`) displays the problem in a more reliable fashion and fails both on MacOS and Linux without this fix.

(as a side note: many thanks for such an in-depth review).

> 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.

In the specific macOS case I'm sure that's the right explanation for the crash. But `Disposer` is a public class that is being widely used throughout JDK and as far as I can see nothing in its contract prevents it from being used in a way I described and illustrated with the second test. Perhaps I was unnecessarily generic in my description.

> 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

Let me drop that test and make the existing one headful.

> 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.

On the issue of the second test, I still believe it is useful:
- it demonstrates a legitimate problem with `Disposer`, albeit admittedly a different one than reported in the bug,
- it shows the problem more clearly as the problematic usage pattern of `Disposer` is contained in the test itself and not buried in JDK code,
- it crashes more reliably,
- it shows that the problem (at least potentially) exists on Linux also,
- it does not have to be marked headful.

-------------

PR: https://git.openjdk.org/jdk/pull/9362



More information about the client-libs-dev mailing list