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

Reply via email to