Re: RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final
On Fri, 28 Jun 2024 09:33:41 GMT, Aleksey Shipilev wrote: > > FWIW, there are more `@Stable` uses here: #19906 > > if you would have a moment to check that, it may be helpful. Thanks. > > That one looks fine: it is set outside of constructor, is intrinsically racy, > and it has AFAICS the recovery paths when we read `null` out of > `MappedEnumCache.generatedSwitch` or `MappedEnumCache.constantsMap`. Thanks Aleksey! - PR Comment: https://git.openjdk.org/jdk/pull/19933#issuecomment-2197287772
Re: RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final
On Thu, 27 Jun 2024 19:30:34 GMT, Aleksey Shipilev wrote: > I was auditing the current uses of `@Stable` before relaxing its barriers > ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an > easy spot. > > `resolvedEnum` is not `final`. So technically publishing the object via data > race can show `resolvedEnum` as `null`, which would break `test()` that does > not expect it. Currently not a practical problem since its safety is covered > by adjacent `final` fields, but should be more precise. > FWIW, there are more `@Stable` uses here: #19906 > if you would have a moment to check that, it may be helpful. Thanks. That one looks fine: it is set outside of constructor, is intrinsically racy, and it has AFAICS the recovery paths when we read `null` out of `MappedEnumCache.generatedSwitch` or `MappedEnumCache.constantsMap`. - PR Comment: https://git.openjdk.org/jdk/pull/19933#issuecomment-2196506663
Re: RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final
On Thu, 27 Jun 2024 19:30:34 GMT, Aleksey Shipilev wrote: > I was auditing the current uses of `@Stable` before relaxing its barriers > ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an > easy spot. > > `resolvedEnum` is not `final`. So technically publishing the object via data > race can show `resolvedEnum` as `null`, which would break `test()` that does > not expect it. Currently not a practical problem since its safety is covered > by adjacent `final` fields, but should be more precise. FWIW, there are more `@Stable` uses here: https://github.com/openjdk/jdk/pull/19906 if you would have a moment to check that, it may be helpful. Thanks. - PR Comment: https://git.openjdk.org/jdk/pull/19933#issuecomment-2196499286
Re: RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final
On Thu, 27 Jun 2024 19:30:34 GMT, Aleksey Shipilev wrote: > I was auditing the current uses of `@Stable` before relaxing its barriers > ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an > easy spot. > > `resolvedEnum` is not `final`. So technically publishing the object via data > race can show `resolvedEnum` as `null`, which would break `test()` that does > not expect it. Currently not a practical problem since its safety is covered > by adjacent `final` fields, but should be more precise. Looks good to me. Marked as reviewed by jlahoda (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19933#pullrequestreview-2147390630 PR Review: https://git.openjdk.org/jdk/pull/19933#pullrequestreview-2147390890
Re: RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final
On Thu, 27 Jun 2024 19:30:34 GMT, Aleksey Shipilev wrote: > I was auditing the current uses of `@Stable` before relaxing its barriers > ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an > easy spot. > > `resolvedEnum` is not `final`. So technically publishing the object via data > race can show `resolvedEnum` as `null`, which would break `test()` that does > not expect it. Currently not a practical problem since its safety is covered > by adjacent `final` fields, but should be more precise. Indeed, the missing final on a stable array usually indicates a level of laziness on the array itself. It's clearly not the case here. - Marked as reviewed by liach (Committer). PR Review: https://git.openjdk.org/jdk/pull/19933#pullrequestreview-2146358637
RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final
I was auditing the current uses of `@Stable` before relaxing its barriers ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an easy spot. `resolvedEnum` is not `final`. So technically publishing the object via data race can show `resolvedEnum` as `null`, which would break `test()` that does not expect it. Currently not a practical problem since its safety is covered by adjacent `final` fields, but should be more precise. - Commit messages: - Fix Changes: https://git.openjdk.org/jdk/pull/19933/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=19933&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8335274 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/19933.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19933/head:pull/19933 PR: https://git.openjdk.org/jdk/pull/19933
