On Mon, 4 Jul 2022 09:31:05 GMT, Maxim Kartashev <[email protected]> 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. I must be missing something here, since you say you saw a crash and this fixes it. Principally you seem to be making access to the deferredRecords list thread safe. But first I'm not sure how where the MT problem is. The code that disposes on the rendering thread is invoked from a dispose() method that was on the Disposer thread. It then waits for that to finish. At that time the Disposer thread is blocked so not doing anything and the render thread is free to add elements to deferredRecords. When it returns the deferredRecords are cleared on the Disposer thread and the rendering thread can't be adding anything to it. Second if it is just about MT access to deferredRecords and pulling elements off that queue then why aren't we getting a Java exception about concurrent modification instead of a crash. And if we do somehow have two threads that end up trying to free the same data - ie both executing the dispose() method of an instance, since they are all (or should) be synchronized AND are supposed to check if they've already been freed .. why isn't that working ? So what am I missing ? BTW when we run this test since it is a headless test we would never have accelerated glyphs .. and the same is true of your new tests .. so I imagine in our test harness and that of others too they'll all always pass. ------------- PR: https://git.openjdk.org/jdk/pull/9362
