Re: [OpenJDK 2D-Dev] RFR: JDK-8263467: Incorrect double-checked locking in sun.java2d.CRenderer [v2]
On Fri, 12 Mar 2021 07:22:26 GMT, Aleksey Shipilev 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?
>
> Aleksey Shipilev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Drop DCL and useless synchronization completely
Marked as reviewed by azvegint (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/2948
Re: [OpenJDK 2D-Dev] RFR: JDK-8263467: Incorrect double-checked locking in sun.java2d.CRenderer [v2]
On Fri, 12 Mar 2021 07:22:26 GMT, Aleksey Shipilev 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?
>
> Aleksey Shipilev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Drop DCL and useless synchronization completely
LGTM
-
Marked as reviewed by prr (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/2948
Re: [OpenJDK 2D-Dev] RFR: JDK-8263467: Incorrect double-checked locking in sun.java2d.CRenderer [v2]
On Fri, 12 Mar 2021 07:22:26 GMT, Aleksey Shipilev 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?
>
> Aleksey Shipilev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Drop DCL and useless synchronization completely
Not now, bot.
-
PR: https://git.openjdk.java.net/jdk/pull/2948
Re: [OpenJDK 2D-Dev] RFR: JDK-8263467: Incorrect double-checked locking in sun.java2d.CRenderer [v2]
> 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?
Aleksey Shipilev has updated the pull request incrementally with one additional
commit since the last revision:
Drop DCL and useless synchronization completely
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/2948/files
- new: https://git.openjdk.java.net/jdk/pull/2948/files/c81b9ea2..d912f559
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=2948=01
- incr: https://webrevs.openjdk.java.net/?repo=jdk=2948=00-01
Stats: 80 lines in 1 file changed: 0 ins; 70 del; 10 mod
Patch: https://git.openjdk.java.net/jdk/pull/2948.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/2948/head:pull/2948
PR: https://git.openjdk.java.net/jdk/pull/2948
