On Tue, 24 Jun 2025 18:34:02 GMT, ExE Boss <d...@openjdk.org> wrote: >> Unsafe throws IAE for misusing static vs instance fields, and it's revealed >> that AtomicXxxFieldUpdaters are using this mechanism to reject static >> fields. This is not a good practice, but we can at least document this so we >> don't accidentally introduce problems. > > src/hotspot/share/prims/unsafe.cpp line 498: > >> 496: if (fs.access_flags().is_static()) { >> 497: offset = -1; >> 498: } else { > > Since `offset` is initialised to `-1`, this can simply do: > Suggestion: > > if (!fs.access_flags().is_static()) {
True. Meanwhile I think I will add a comment to indicate this intends to fail fast when a matching static field is encountered. > src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 1070: > >> 1068: * >> 1069: * @throws NullPointerException if the field is {@code null} >> 1070: * @throws IllegalArgumentException if the field is static > > Maybe these checks could be done in **Java** like: > > // implicit null check: > if ((f.getModifiers() & ACC_STATIC) != 0) { > throw new IllegalArgumentException("Field is static"); > } > > and for the `staticField(Offset/Base)` methods: > > // implicit null check: > if ((f.getModifiers() & ACC_STATIC) == 0) { > throw new IllegalArgumentException("Field is not static"); > } Don't think anyone is willing to change code here... > src/java.base/share/classes/jdk/internal/misc/Unsafe.java line 1092: > >> 1090: * in class {@code c}, i.e., if {@code >> c.getDeclaredField(name)} >> 1091: * would throw {@code java.lang.NoSuchFieldException}, or >> if such >> 1092: * a field is static > > This is actually not the case, the native code only checks whether the field > exists, not whether it’s also not `static`: > https://github.com/openjdk/jdk/blob/cbcf401170e0600e48ef74770eaa47c84c7e50b0/src/hotspot/share/prims/unsafe.cpp#L492-L503 > > Suggestion: > > * would throw {@code java.lang.NoSuchFieldException}. Hmm, didn't know that field stream included static fields ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25945#discussion_r2164666239 PR Review Comment: https://git.openjdk.org/jdk/pull/25945#discussion_r2164623978 PR Review Comment: https://git.openjdk.org/jdk/pull/25945#discussion_r2164623202