On Fri, 12 Mar 2021 03:19:12 GMT, Sergey Bylokhov <s...@openjdk.org> wrote:
>> SonarCloud reports multiple incorrect double-checked locking cases in >> `sun.java2d.CRenderer`. For example: >> >> Arc2D arcToShape; >> >> ... >> if (arcToShape == null) { >> synchronized (this) { >> if (arcToShape == null) { >> arcToShape = new Arc2D.Float(); >> } >> } >> } >> >> `Arc2D` contains fields that are not `final`. `arcToShape` is not >> `volatile`. This makes it an incorrect DCL. >> This code is used by Mac OS, do it would probably blow up some time later on >> M1. >> >> This is the candidate fix that preserves the semantics. I am doing this fix >> blindly so far, without testing on real Mac OS. But maybe there is no need >> to do any of this, because the setter methods overwrite the contents of all >> these objects under their own sync. So, maybe we can just allocate those >> objects without DCL-backed caching and pass them in? > > I guess "Incorrect double-checked" is a too strict description of this code, > it just tried to eliminate the creation of one object and do not care about > the content of that object, since it is immediately overridden. > > I think It will be more clear to just eliminate this DCL since overhead and > code complexity seems too much to just cache five objects. > > Probably it was important in past(jdk6 and below) when this renderer was used > for onscreen rendering. Currently, it is used for printing(if actually used > at all). Ok, I removed the "DCL" from that code and used the objects directly. AFAICS, synchronization is redundant on those local objects. What would be a good test for this change? ------------- PR: https://git.openjdk.java.net/jdk/pull/2948