On Thu, 20 Aug 2026 23:40:49 GMT, Kevin Rushforth <[email protected]> wrote:

>> This PR creates a WeakReferenceWrapper object to replace direct uses of 
>> WeakReference in controls where the referent is a user-supplied object of an 
>> unknown type.
>> 
>> As noted in JEP 401, which is now integrated into JDK 28, "The garbage 
>> collection APIs in java.lang.ref ... do not allow developers to manually 
>> manage value objects in the heap. Attempts to create Reference objects for 
>> value objects throw IdentityException at run time."
>> 
>> Several core JDK classes such as all of the primitive wrappers (e.g., 
>> `Integer`, `Character`), `Optional`, `LocalDateTime`, and a few others are 
>> now value types if JDK 28 is run with the `--enable-preview` option.
>> 
>> The `ListView`, `ComboBox`, `TableView`, and `TreeTableView` controls take a 
>> parameterized item type and hold items of that type. The following places in 
>> the implementation create weak references to an item. If that item type is a 
>> value class -- meaning that it does not have identity -- creating the 
>> `WeakReference` fails.
>> 
>> As noted in the JBS issue, there are 3 cases to consider.
>> 
>> 1. `SelectedItemsReadOnlyObservableList<E>` -- `E` is the item type (created 
>> by `MultipleSelectionModelBase<T>`) : `ListView`, `TableView`, `ComboBox` 
>> (due to its skin creating a `ListView<T>`) -- replace with 
>> `WeakReferenceWrapper`
>> 
>> 2. `TablePosition<S,T>` -- `S` is the item type : `TableView` -- the 
>> reference is unused, so I removed it
>> 
>> 3. `TableCell<S,T>` and `TreeTableCell<S,T>` -- `S` is the item type : 
>> `TableView`, `TreeTableView` -- replace with `WeakReferenceWrapper`
>> 
>> The new `WeakReferenceWrapper` class takes a referent of any type and either 
>> creates a WeakReference (if it has identity) or directly stores the 
>> reference (if it is null or does not have identity). I added a test for the 
>> wrapper.
>> 
>> All of the controls tests pass with this fix. I did three test runs as 
>> follows:
>> 
>> 1. JDK 25
>> 2. JDK 28 without `--enable-preview`
>> 3. JDK 28 with `--enable-preview`
>> 
>> Without the fix, 31 controls tests fails on the 3rd run.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Kevin Rushforth has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   document that WeakReferenceWrapper might be modified or eliminated in the 
> future

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/WeakReferenceWrapper.java
 line 60:

> 58:  * @param <T> the type of the referent
> 59:  */
> 60: public class WeakReferenceWrapper<T> {

Personally, I’d use a sealed interface with a strong and weak implementation 
instead, something like:

import java.lang.invoke.MethodHandle;
import java.util.Objects;
import static java.lang.invoke.MethodHandles.publicLookup;
import static java.lang.invoke.MethodType.methodType;

import org.jspecify.annotations.*;

@NullMarked
sealed interface WeakRefWrapper<T> {
        private static boolean hasIdentity(final @Nullable Object obj) {
                if (obj == null) {
                        return false;
                }

                final @Namespace class Holder {
                        static final @Nullable MethodHandle 
OBJECTS_HAS_IDENTITY;
                        static {
                                MethodHandle hasIdentity = null;
                                try {
                                        hasIdentity = publicLookup().findStatic(
                                                Objects.class,
                                                "hasIdentity",
                                                methodType(boolean.class, 
Object.class)
                                        );
                                } catch (final IllegalAccessException ex) {
                                        // `Exceptions::fromROE` 
(fromReflectiveOperationException) behaves mostly
                                        // like 
`java.lang.invoke.MethodHandleNatives::mapLookupExceptionToError`
                                        throw Exceptions.fromROE(ex);
                                } catch (NoSuchMethodException _) {
                                        // ok
                                }
                                OBJECTS_HAS_IDENTITY = hasIdentity;
                        }
                }

                final var hasIdentity = Holder.OBJECTS_HAS_IDENTITY;
                if (hasIdentity == null) {
                        return true;
                }

                try {
                        return (boolean) hasIdentity.invokeExact(obj);
                } catch (final Throwable t) {
                        // `Exceptions::asUnchecked` rethrows the `Throwable`
                        // as if it were an unchecked exception
                        throw Exceptions.asUnchecked(t);
                }
        }

        public static <T extends @Nullable Object> WeakRefWrapper<T> of(final T 
value) {
                if (hasIdentity(value)) {
                        return new WeakImpl<>(value);
                } else {
                        return new StrongImpl<>(value);
                }
        }

        public @Nullable T get();

        private static final class WeakImpl<T> extends WeakReference<T>
                implements WeakRefWrapper<T> {

                WeakImpl(final T referent) {
                        super(Objects.requireNonNull(referent));
                }
        }

        private static final class StrongImpl<T extends @Nullable Object>
                implements WeakRefWrapper<T> {

                private final T value;
                StrongImpl(final T value) {
                        this.value = value;
                }

                @Override
                public T get() {
                        return this.value;
                }
        }
}

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/2250#discussion_r3839380567

Reply via email to