On Thu, 6 Aug 2026 20:34:13 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). modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/WeakReferenceWrapper.java line 40: > 38: * In the case of a value object, the referent is never collected, so it > is only > 39: * suitable for uses that do not rely on the object being placed onto a > reference > 40: * queue. I might suggest to include the JBS number for when we need to undo / redo things due to inevitable change in the value objects JEP (and possibly a link to the JEP itself). Perhaps also say a couple of words about the fact that this class should not exist had they decided to make the WeakReference implementation handle this case transparently. modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/WeakReferenceWrapper.java line 44: > 42: * @param <T> the type of the referent > 43: */ > 44: public class WeakReferenceWrapper<T> { An application or a library should not be forced to do this, in my opinion. WeakReference(T) should work as before - in case of a value object it should hold the reference indefinitely because it's the same behavior as before. It may work differently with the WeakReference(T,ReferenceQueue) constructor which is ok because it would affect a much smaller space of use cases. It is probably ok to do it right now, to avoid javafx breaking with the value objects preview enabled. modules/javafx.controls/src/main/java/javafx/scene/control/TableCell.java line 635: > 633: private boolean isFirstRun = true; > 634: > 635: private WeakReferenceWrapper<S> oldRowItemRef; While it is ok to apply this workaround (WeakReferenceWrapper) here, this kind of change in the WeakReference behavior is just awful: we should never force the application developers (r a third party library developers) to make a change like this. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2250#discussion_r3787295258 PR Review Comment: https://git.openjdk.org/jfx/pull/2250#discussion_r3787235794 PR Review Comment: https://git.openjdk.org/jfx/pull/2250#discussion_r3787332708
