RFR: 8349004: DatePicker: NPE in show() when initialized in a background thread

Kevin Rushforth kcr at openjdk.org
Wed Feb 12 23:13:14 UTC 2025


On Wed, 12 Feb 2025 19:45:32 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

> ## Root Cause
> 
> Focus is being requested in show(), even a background thread.
> 
> ## Solution
> 
> Do not request focus if in a background thread.

I'm not sure this fix is sufficient in all cases.

As I mentioned in the bug report, I think we need to consider disallowing calling `ComboBoxBaseSkin::show` (including the overridden methods in the subclasses) from a background thread. At a minimum, we need to ensure that the `ComboBoxBaseSkin::show` implementations never call any of the `PopupWindow::show` method. In looking at the code, it seems possible that there are some cases where it might.

It is already a documented (and enforced) error to call Window::show -- and by extension, the various `PopupWindow::show` methods on a background thread. I note that the `PopupWindow::show` methods are not documented to throw, but they do throw when they call the no-arg `Window::show` method, which they do if the parent Window is visible, so a documented, fail-fast exception check is best.

So I think we need to do one of two things in each of the three overridden `ComboBoxBaseSkin::show` methods (`ComboBoxPopupControl`, `ColorPickerSkin`, `DatePickerSkin`).

1. Change the spec and implementation to throw an exception if `ComboBoxBaseSkin::show` is called on a thread other than the FX Application Thread
2. Go with the currently proposed fix, but also ensure that our implementations  of`ComboBoxBaseSkin::show` don't call any of the `PopupWindow::show` methods on a background thread

II lean toward option 1, since I can't think of a good reason for an app to call show() from a background thread.

Either way, I want to see a follow-up bug filed to document that the three `PopupWindow::show` methods must be called on the FX app thread (with a check to fail fast if they don't so that it is caught in all cases).

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

PR Review: https://git.openjdk.org/jfx/pull/1708#pullrequestreview-2613431873


More information about the openjfx-dev mailing list