In Types.arraySuperType(), SonarCloud reports:
Remove this dangerous instance of double-checked locking. Indeed, the `arraySuperType` is not `volatile`, while `IntersectionClassType` has non-`final` fields (both in itself and in superclasses). This is an incorrect DCL. ------------- Commit messages: - 8263995: Incorrect double-checked locking in Types.arraySuperType() Changes: https://git.openjdk.java.net/jdk/pull/3129/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3129&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263995 Stats: 8 lines in 1 file changed: 3 ins; 0 del; 5 mod Patch: https://git.openjdk.java.net/jdk/pull/3129.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3129/head:pull/3129 PR: https://git.openjdk.java.net/jdk/pull/3129 |
On Mon, 22 Mar 2021 18:19:39 GMT, Aleksey Shipilev <[hidden email]> wrote:
> In Types.arraySuperType(), SonarCloud reports: > Remove this dangerous instance of double-checked locking. > > Indeed, the `arraySuperType` is not `volatile`, while `IntersectionClassType` has non-`final` fields (both in itself and in superclasses). This is an incorrect DCL. I'm a bit surprised to see use of volatile/synchronized here as I'm pretty sure that javac is *not* thread safe. It seems like the synchronized block has been there from a long time (e.g. it was there since the dawn of OpenJDK :-)). I wonder if a better fix would be simply to drop synchronization and volatileness. ------------- PR: https://git.openjdk.java.net/jdk/pull/3129 |
In reply to this post by Aleksey Shipilev-5
> In Types.arraySuperType(), SonarCloud reports:
> Remove this dangerous instance of double-checked locking. > > Indeed, the `arraySuperType` is not `volatile`, while `IntersectionClassType` has non-`final` fields (both in itself and in superclasses). This is an incorrect DCL. Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: Drop synchronization and volatileness ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/3129/files - new: https://git.openjdk.java.net/jdk/pull/3129/files/e8092ca6..aeee7536 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3129&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3129&range=00-01 Stats: 13 lines in 1 file changed: 0 ins; 7 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/3129.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3129/head:pull/3129 PR: https://git.openjdk.java.net/jdk/pull/3129 |
In reply to this post by Maurizio Cimadamore-2
On Mon, 22 Mar 2021 18:31:01 GMT, Maurizio Cimadamore <[hidden email]> wrote:
>> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: >> >> Drop synchronization and volatileness > > I'm a bit surprised to see use of volatile/synchronized here as I'm pretty sure that javac is *not* thread safe. > It seems like the synchronized block has been there from a long time (e.g. it was there since the dawn of OpenJDK :-)). I wonder if a better fix would be simply to drop synchronization and volatileness. > I'm a bit surprised to see use of volatile/synchronized here as I'm pretty sure that javac is _not_ thread safe. > It seems like the synchronized block has been there from a long time (e.g. it was there since the dawn of OpenJDK :-)). I wonder if a better fix would be simply to drop synchronization and volatileness. Yeah, I just did the most obvious semantics-preserving thing in this first patch. See new commit that drops `synchronized` instead as to not confuse both humans and tools about this. Still passes `langtools:tier1`. ------------- PR: https://git.openjdk.java.net/jdk/pull/3129 |
In reply to this post by Aleksey Shipilev-5
On Mon, 22 Mar 2021 19:06:00 GMT, Aleksey Shipilev <[hidden email]> wrote:
>> In Types.arraySuperType(), SonarCloud reports: >> Remove this dangerous instance of double-checked locking. >> >> Indeed, the `arraySuperType` is not `volatile`, while `IntersectionClassType` has non-`final` fields (both in itself and in superclasses). This is an incorrect DCL. > > Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: > > Drop synchronization and volatileness Looks good to me! ------------- Marked as reviewed by jlahoda (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3129 |
In reply to this post by Aleksey Shipilev-5
On Mon, 22 Mar 2021 19:06:00 GMT, Aleksey Shipilev <[hidden email]> wrote:
>> In Types.arraySuperType(), SonarCloud reports: >> Remove this dangerous instance of double-checked locking. >> >> Indeed, the `arraySuperType` is not `volatile`, while `IntersectionClassType` has non-`final` fields (both in itself and in superclasses). This is an incorrect DCL. > > Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision: > > Drop synchronization and volatileness Looks good ------------- Marked as reviewed by mcimadamore (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3129 |
In reply to this post by Aleksey Shipilev-5
On Mon, 22 Mar 2021 18:19:39 GMT, Aleksey Shipilev <[hidden email]> wrote:
> In Types.arraySuperType(), SonarCloud reports: > Remove this dangerous instance of double-checked locking. > > Indeed, the `arraySuperType` is not `volatile`, while `IntersectionClassType` has non-`final` fields (both in itself and in superclasses). This is an incorrect DCL. This pull request has now been integrated. Changeset: c087f3ed Author: Aleksey Shipilev <[hidden email]> URL: https://git.openjdk.java.net/jdk/commit/c087f3ed Stats: 8 lines in 1 file changed: 0 ins; 4 del; 4 mod 8263995: Incorrect double-checked locking in Types.arraySuperType() Reviewed-by: mcimadamore, jlahoda ------------- PR: https://git.openjdk.java.net/jdk/pull/3129 |
Free forum by Nabble | Edit this page |