On Wed, 17 Jun 2026 15:13:59 GMT, Chen Liang <[email protected]> wrote:
>> The checks for strict statics for VarHandle was never implemented. Luckily >> we can reuse LazyInitializingVarHandle so the patch is not too complex. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains five additional commits since > the last revision: > > - Fix missing trusted final field query > - Merge branch 'lworld' of https://github.com/openjdk/valhalla into > fix/lw-strict-static-vh > - Restrict write access for IMPL_LOOKUP VH on strict final > - Merge branch 'lworld' of https://github.com/openjdk/valhalla into > fix/lw-strict-static-vh > - Missing strict static validation for VarHandle Had a few more questions (inline). src/java.base/share/classes/java/lang/invoke/LazyInitializingVarHandle.java line 116: > 114: return target.getMethodHandle(accessMode); > 115: > 116: return super.getMethodHandleUncached(accessMode); `target.getMethodHandle` goes to the target, but `super.getMethodHandleUncached` goes to our `vform`, which is the target's `vform`. What's the difference? (besides the lack of caching in the targets `methodHandleTable` in the latter case) Both will end up returning the same method handle right? src/java.base/share/classes/java/lang/invoke/VarHandles.java line 163: > 161: } > 162: else if (type == boolean.class) { > 163: return maybeAdapt(f.isFinal() && !isWriteAllowedOnFinalFields Shouldn't we keep the `maybeAdapt` here? It's used for testing indirect var handles. src/java.base/share/classes/java/lang/invoke/X-VarHandle.java.template line 619: > 617: @ForceInline > 618: static boolean compareAndSet(VarHandle ob, $type$ expected, > $type$ value) { > 619: FieldStaticReadWrite handle = (FieldStaticReadWrite) > ob.onStaticFieldAccess(true, true); CompareAndSet do both a read and write, so here you pass `true, true`. This will end up calling `notifyStrictStaticAccess` which then clears the 'unset' bit of the field for writes. However, a CAS is not guaranteed to do a write. Isn't that problematic? The VM would consider the field initialized after a failed CAS. Would be nice if you could add a test for that. ------------- PR Review: https://git.openjdk.org/valhalla/pull/2550#pullrequestreview-4525047090 PR Review Comment: https://git.openjdk.org/valhalla/pull/2550#discussion_r3435991095 PR Review Comment: https://git.openjdk.org/valhalla/pull/2550#discussion_r3435943962 PR Review Comment: https://git.openjdk.org/valhalla/pull/2550#discussion_r3435934693
