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
