JDK-8181930 : Adding a null icon to a Stage prevents application startup
  • Type: Bug
  • Component: javafx
  • Sub-Component: scenegraph
  • Affected Version: 8,9,10
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: x86
  • Submitted: 2017-06-11
  • Updated: 2017-08-06
  • Resolved: 2017-08-06
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 10
10Fixed
Description
FULL PRODUCT VERSION :
>java -version
java version "1.8.0_131"
Java(TM) SE Runtime Environment (build 1.8.0_131-b11)
Java HotSpot(TM) 64-Bit Server VM (build 25.131-b11, mixed mode)

ADDITIONAL OS VERSION INFORMATION :
> ver
Microsoft Windows [Version 10.0.14393]


A DESCRIPTION OF THE PROBLEM :
It is possible to add a null value to a Stage's icon list. This raises a NullPointerException that is internally caught by the JavaFX API and never reaches the programmer's level. Later when attempting to make the stage visible, the application will crash with a InvocationTargetException (caused by a RuntimeException caused by a NullPointerException).

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
Create a new JavaFX application.
In the start() method of the application's main class, add a null value to the Stage's icons list.
Make the stage visible.

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
We should expect either of this result :

* Either an exception is raised immediately when attempting to insert the null value.

* Or the null value is ignored by the underlying stage code.
ACTUAL -
A NullPointerException is indeed raised by the JavaFX API but it is caught internally at line 164 of class com.sun.javafx.collections.ListListenerHelper<E>

Later when the Stage is made visible, another exception is raised.

ERROR MESSAGES/STACK TRACES THAT OCCUR :
// 1st exception caugh internally by the JavaFX API.
Exception in thread "JavaFX Application Thread" java.lang.NullPointerException
finally
	at javafx.stage.Stage$4.onChanged(Stage.java:696)
	at com.sun.javafx.collections.TrackableObservableList.lambda$new$19(TrackableObservableList.java:45)
	at com.sun.javafx.collections.ListListenerHelper$SingleChange.fireValueChangedEvent(ListListenerHelper.java:164)
	at com.sun.javafx.collections.ListListenerHelper.fireValueChangedEvent(ListListenerHelper.java:73)
	at javafx.collections.ObservableListBase.fireChange(ObservableListBase.java:233)
	at javafx.collections.ListChangeBuilder.commit(ListChangeBuilder.java:482)
	at javafx.collections.ListChangeBuilder.endChange(ListChangeBuilder.java:541)
	at javafx.collections.ObservableListBase.endChange(ObservableListBase.java:205)
	at javafx.collections.ModifiableObservableListBase.add(ModifiableObservableListBase.java:155)
	at java.util.AbstractList.add(AbstractList.java:108)
	at stagenull.Main.start(Main.java:25)
	at com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$162(LauncherImpl.java:863)
	at com.sun.javafx.application.PlatformImpl.lambda$runAndWait$175(PlatformImpl.java:326)
	at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294)
	at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at com.sun.glass.ui.win.WinApplication.lambda$null$148(WinApplication.java:191)
	at java.lang.Thread.run(Thread.java:748)


// 2nd exception raised when attempting to make the stage visible
Exception in Application start method
java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at com.sun.javafx.application.LauncherImpl.launchApplicationWithArgs(LauncherImpl.java:389)
	at com.sun.javafx.application.LauncherImpl.launchApplication(LauncherImpl.java:328)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at sun.launcher.LauncherHelper$FXHelper.main(LauncherHelper.java:767)
Caused by: java.lang.RuntimeException: Exception in Application start method
	at com.sun.javafx.application.LauncherImpl.launchApplication1(LauncherImpl.java:917)
	at com.sun.javafx.application.LauncherImpl.lambda$launchApplication$155(LauncherImpl.java:182)
	at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
	at javafx.stage.Stage.impl_visibleChanged(Stage.java:1189)
	at javafx.stage.Window$9.invalidated(Window.java:895)
	at javafx.beans.property.BooleanPropertyBase.markInvalid(BooleanPropertyBase.java:109)
	at javafx.beans.property.BooleanPropertyBase.set(BooleanPropertyBase.java:144)
	at javafx.stage.Window.setShowing(Window.java:922)
	at javafx.stage.Window.show(Window.java:937)
	at javafx.stage.Stage.show(Stage.java:259)
	at stagenull.Main.start(Main.java:33)
	at com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$162(LauncherImpl.java:863)
	at com.sun.javafx.application.PlatformImpl.lambda$runAndWait$175(PlatformImpl.java:326)
	at com.sun.javafx.application.PlatformImpl.lambda$null$173(PlatformImpl.java:295)
	at java.security.AccessController.doPrivileged(Native Method)
	at com.sun.javafx.application.PlatformImpl.lambda$runLater$174(PlatformImpl.java:294)
	at com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:95)
	at com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at com.sun.glass.ui.win.WinApplication.lambda$null$148(WinApplication.java:191)
	... 1 more
Exception running application stagenull.Main

REPRODUCIBILITY :
This bug can be reproduced always.

---------- BEGIN SOURCE ----------
The following code makes the error happen all the time:

import javafx.application.Application;
import javafx.stage.Stage;

public class Main extends Application {

    @Override
    public void start(Stage primaryStage) {
        try {
            System.out.println("start try");
            primaryStage.getIcons().add(null); // NullPointerException caught internally.
            System.out.println("end try");  // Always happen.
        } catch (Exception e) {
            System.out.println("error caught"); // Never happen.
        } finally {
            System.out.println("finally");
        }
        primaryStage.setTitle("Hello World!");
        primaryStage.show(); // InvocationTargetException <- RuntimeException <- NullPointerException
    }

    public static void main(String[] args) {
        launch(args);
    }
}


---------- END SOURCE ----------

CUSTOMER SUBMITTED WORKAROUND :
Simply never insert a null value to the Stage's icon list. However as the error is not raised to the programmer's level it may be difficult to catch when resources such as icons are loaded programmatically.

So there should be 2 possible workaround for this bug:

* prevent null values from being added to the stage's icons list. The List interface does not prevent from addition null objects so in theory the null value is valid but on the other hand JavaFX has already this behavior when adding a null child to a parent. For example:

try {
    StackPane root = new StackPane();
    root.getChildren().add(null);
} catch (Exception e) {
     System.out.println("error caught"); // Happens all the time.
}

This code raises an InvocationTargetException (caused by a RuntimeException caused by a NullPointerException) immediately and it reach's the programmer's level. We could change the icon observable list in the Stage class to behave in a similar fashion.

* Or the handling of the icons' list should be modified to ignore null values completely. 
This would happen in JavaFX 8_131 at lines 696 and 1189 of the Stage class where, in both cases, the content of the list is being handled in order to get the peer native images.

Current unsafe code:

List<Object> platformImages = new ArrayList<Object>();
for (Image icon : icons) {
    platformImages.add(icon.impl_getPlatformImage());
}

Should become:

List<Object> platformImages = new ArrayList<Object>();
for (Image icon : icons) {
    if (icon == null) {
       continue;
    }
    platformImages.add(icon.impl_getPlatformImage());
}

Or by upgrading this code to JDK 8 streams and filter to prevent null values:

List<Object> platformImages = icons.stream()
                .filter(Objects::nonNull)
                .map(Image::impl_getPlatformImage)
                .collect(Collectors.toList());




Comments
Changeset: http://hg.openjdk.java.net/openjfx/10-dev/rt/rev/1bedda819f21
06-08-2017

Looks fine to me, +1.
04-08-2017

Hi Kevin & Jonathan, Thanks for the pointers. Throwing NPE from the listener onChanged() method is not fixing the issue. Exception thrown from the listener onChanged() gets caught internally and passed to ListListenerHelper (ListListenerHelper::fireValueChangedEvent()), hence the NPE does not reach the application code. In a similar way to ObservableList Parent::children, wrapping TrackableObservableList inside a VetoableListDecorator fixes the issue. Request you to review the updated webrev. http://cr.openjdk.java.net/~arapte/fx/8181930/webrev.01/
03-08-2017

Might the existing onChanged listener be a good place to put it? It would need to be tested.
03-08-2017

I agree with Kevin - this approach feels fragile. Another option to consider is to simply add a listener to the icons list such that a null value results in a runtime exception that is surfaced to the user. I would guess that this is the best option simply in that the user should never knowingly add null to the icons list, so it must be an error on their part (perhaps an incorrect resource reference, etc).
03-08-2017

Relying on the implementation to always call set or add for all methods that add a new element to the list seems fragile. Have you considered wrapping the list in a VetoableListDecorator which is used elsewhere for ObservableLists that can veto proposed changes? See Parent::getChildren for example. I'd like Jonathan to weigh in on this.
03-08-2017

Hi Chien, Hi Kevin, Request you to review the fix for JDK 10. Webrev: http://cr.openjdk.java.net/~arapte/fx/8181930/webrev.00/ Issue: As mentioned by Kevin, the bug is just that we do not report the error in a timely manner. Fix: Overridden two methods of TrackableObservableList when creating the icon. All of the methods add, addAll set, setAll finally call one of the two overridden methods. Verification: Verified that all the variants of add & set APIs throw NPE. Added a new test. No existing test fails due to this change.
02-08-2017

Confirmed that this bug has been there since at least 8 GA. As such I am removing the regression keyword (and the 9-bp label). I note that this is basically the result of an application error, and the bug is just that we do not report the error in a timely manner.
13-06-2017

The issue is also reproducible with previous builds of 8, on Windows 7 x64 Tested with : 1.8.0-ea-b87 1.8.0_112-ea-b01 8 GA 8 u11 8 u20
13-06-2017

This should be fixed in JDK 10, with a possible backport to a 9 update release.
12-06-2017

When ran attached test case, got "NPE" caught at programmer's level only in 8GA and 8u11. Windows 10 ======== 8 GA : Pass 8u11 : Pass 8u20 : Fail <--- Issue reproduced here 8u60 : Fail 8u121 : Fail 8u131 : Fail 9-173 : Fail ========
12-06-2017