JDK-8240506 : TextFieldSkin/Behavior: misbehavior on switching skin
  • Type: Bug
  • Component: javafx
  • Sub-Component: controls
  • Affected Version: openjfx13
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: windows_10
  • CPU: x86_64
  • Submitted: 2020-02-28
  • Updated: 2021-07-30
  • Resolved: 2021-07-25
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.
Other
openjfx18Fixed
Related Reports
Blocks :  
Relates :  
Relates :  
Relates :  
Description
ADDITIONAL SYSTEM INFORMATION :
Windows 10
Java 13.0.2 (Azul jdk)
JavaFX 13.0.2
JMetro 11.6.7
Gradel: 6.2.1

A DESCRIPTION OF THE PROBLEM :
I seems like when switching between skins, the dispose method is not cleaning up listeners properly. I found this while working with JMetro (https://github.com/JFXtras/jfxtras-styles/tree/master/src/jmetro). I started my application using the default theme, and then when I tried to switch to JMetro, I got an error. I reported this to the developer of JMetro (https://github.com/JFXtras/jfxtras-styles/issues/144), who believes that it is actually a JavaFX issue.

STEPS TO FOLLOW TO REPRODUCE THE PROBLEM :
1. Start the Application without using JMetro
2. Switch to using JMetro

EXPECTED VERSUS ACTUAL BEHAVIOR :
EXPECTED -
UI should change to JMetro theme, with no errors.
ACTUAL -
UI does look like it changes to JMetro, but the following error occurs:

Exception in thread "JavaFX Application Thread" java.lang.NullPointerException
	at javafx.controls/javafx.scene.control.skin.TextFieldSkin.updateSelection(TextFieldSkin.java:740)
	at javafx.controls/javafx.scene.control.skin.TextFieldSkin.lambda$new$3(TextFieldSkin.java:233)
	at javafx.base/com.sun.javafx.binding.ExpressionHelper$SingleInvalidation.fireValueChangedEvent(ExpressionHelper.java:136)
	at javafx.base/com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80)
	at javafx.base/javafx.beans.property.ObjectPropertyBase.fireValueChangedEvent(ObjectPropertyBase.java:106)
	at javafx.base/javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:113)
	at javafx.base/javafx.beans.property.ObjectPropertyBase$Listener.invalidated(ObjectPropertyBase.java:234)
	at javafx.base/com.sun.javafx.binding.ExpressionHelper$SingleInvalidation.fireValueChangedEvent(ExpressionHelper.java:136)
	at javafx.base/com.sun.javafx.binding.ExpressionHelper.fireValueChangedEvent(ExpressionHelper.java:80)
	at javafx.base/javafx.beans.binding.ObjectBinding.invalidate(ObjectBinding.java:170)
	at javafx.graphics/javafx.scene.text.Text.doGeomChanged(Text.java:847)
	at javafx.graphics/javafx.scene.text.Text$1.doGeomChanged(Text.java:159)
	at javafx.graphics/com.sun.javafx.scene.shape.TextHelper.geomChangedImpl(TextHelper.java:106)
	at javafx.graphics/com.sun.javafx.scene.NodeHelper.geomChanged(NodeHelper.java:137)
	at javafx.graphics/javafx.scene.text.Text$9.invalidated(Text.java:831)
	at javafx.base/javafx.beans.property.ObjectPropertyBase.markInvalid(ObjectPropertyBase.java:112)
	at javafx.base/javafx.beans.property.ObjectPropertyBase.set(ObjectPropertyBase.java:147)
	at javafx.graphics/javafx.css.StyleableObjectProperty.set(StyleableObjectProperty.java:82)
	at javafx.graphics/javafx.css.StyleableObjectProperty.applyStyle(StyleableObjectProperty.java:68)
	at javafx.graphics/javafx.scene.CssStyleHelper.transitionToState(CssStyleHelper.java:787)
	at javafx.graphics/javafx.scene.Node.doProcessCSS(Node.java:9660)
	at javafx.graphics/javafx.scene.Node$1.doProcessCSS(Node.java:472)
	at javafx.graphics/com.sun.javafx.scene.NodeHelper.processCSSImpl(NodeHelper.java:192)
	at javafx.graphics/com.sun.javafx.scene.NodeHelper.processCSS(NodeHelper.java:145)
	at javafx.graphics/javafx.scene.Parent.doProcessCSS(Parent.java:1399)
	at javafx.graphics/javafx.scene.Parent$1.doProcessCSS(Parent.java:125)
	at javafx.graphics/com.sun.javafx.scene.ParentHelper.processCSSImpl(ParentHelper.java:98)
	at javafx.graphics/com.sun.javafx.scene.NodeHelper.processCSS(NodeHelper.java:145)
	at javafx.graphics/javafx.scene.Parent.doProcessCSS(Parent.java:1399)
	at javafx.graphics/javafx.scene.Parent$1.doProcessCSS(Parent.java:125)
	at javafx.graphics/com.sun.javafx.scene.ParentHelper.processCSSImpl(ParentHelper.java:98)
	at javafx.controls/com.sun.javafx.scene.control.ControlHelper.superProcessCSSImpl(ControlHelper.java:63)
	at javafx.controls/com.sun.javafx.scene.control.ControlHelper.superProcessCSS(ControlHelper.java:55)
	at javafx.controls/javafx.scene.control.Control.doProcessCSS(Control.java:886)
	at javafx.controls/javafx.scene.control.Control$1.doProcessCSS(Control.java:89)
	at javafx.controls/com.sun.javafx.scene.control.ControlHelper.processCSSImpl(ControlHelper.java:67)
	at javafx.graphics/com.sun.javafx.scene.NodeHelper.processCSS(NodeHelper.java:145)
	at javafx.graphics/javafx.scene.Parent.doProcessCSS(Parent.java:1399)
	at javafx.graphics/javafx.scene.Parent$1.doProcessCSS(Parent.java:125)
	at javafx.graphics/com.sun.javafx.scene.ParentHelper.processCSSImpl(ParentHelper.java:98)
	at javafx.graphics/com.sun.javafx.scene.NodeHelper.processCSS(NodeHelper.java:145)
	at javafx.graphics/javafx.scene.Node.processCSS(Node.java:9542)
	at javafx.graphics/javafx.scene.Scene.doCSSPass(Scene.java:569)
	at javafx.graphics/javafx.scene.Scene$ScenePulseListener.pulse(Scene.java:2469)
	at javafx.graphics/com.sun.javafx.tk.Toolkit.lambda$runPulse$2(Toolkit.java:412)
	at java.base/java.security.AccessController.doPrivileged(AccessController.java:391)
	at javafx.graphics/com.sun.javafx.tk.Toolkit.runPulse(Toolkit.java:411)
	at javafx.graphics/com.sun.javafx.tk.Toolkit.firePulse(Toolkit.java:438)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:563)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.pulse(QuantumToolkit.java:543)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.pulseFromQueue(QuantumToolkit.java:536)
	at javafx.graphics/com.sun.javafx.tk.quantum.QuantumToolkit.lambda$runToolkit$11(QuantumToolkit.java:342)
	at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication._runLoop(Native Method)
	at javafx.graphics/com.sun.glass.ui.win.WinApplication.lambda$runLoop$3(WinApplication.java:174)
	at java.base/java.lang.Thread.run(Thread.java:830)

---------- BEGIN SOURCE ----------
package ca.ewert.jmetrotest;

import javafx.application.Application;
import javafx.geometry.Insets;
import javafx.scene.Scene;
import javafx.scene.control.*;
import javafx.scene.layout.*;
import javafx.stage.Stage;
import jfxtras.styles.jmetro.*;

public class JMetroTestApplication extends Application {

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

  private Scene scene;

  @Override
  public void start(Stage primaryStage) {
    String javaVersion = System.getProperty("java.version");
    String javafxVersion = System.getProperty("javafx.version");
    Label label = new Label("Hello, JavaFX " + javafxVersion + ", running on Java " + javaVersion + ".");
    TextField textField = new TextField();
    CheckBox checkBox = new CheckBox("Hello JavaFx");
    Button button = new Button("Switch to JMetro");
    button.setOnAction(event -> {
      this.switchToJMetro();
    });

    VBox vBox = new VBox();
    vBox.setPadding(new Insets(10.0));
    vBox.setSpacing(5.0);
    vBox.getChildren().addAll(label, textField, checkBox, button);
    this.scene = new Scene(vBox, 800, 600);
    //this.switchToJMetro();
    primaryStage.setScene(scene);
    primaryStage.setTitle("JMetro Test");
    primaryStage.show();
  }

  private void switchToJMetro() {
    System.out.println("Switching to JMetro");
    JMetro jMetro = new JMetro(Style.LIGHT);
    jMetro.setScene(this.scene);
  }
}
---------- END SOURCE ----------

FREQUENCY : always



Comments
Changeset: 653e24ba Author: Jeanette Winzenburg <fastegal@openjdk.org> Date: 2021-07-25 11:34:38 +0000 URL: https://git.openjdk.java.net/jfx/commit/653e24baef01c52ec11efab255bf506ee55e4d2a
25-07-2021

post-poned until JDK-8258777 is fixed, using the new api will simplify (hopefully ;) the fix to the last group: - change manual registration of both change- and invalidationListeners to skin api
12-01-2021

currently, TextField is excluded from both Skin- and BehaviorMemoryLeakTest - including it in both makes both tests fail memory leak in TextInputControlBehavior: - listener accidentally added twice (removed once) memory leak TextFieldBehavior: - listeners to scene/focusOwner property not removed, - temporary inputMap (for keypad mappings) not disposed memory leak in TextInputControlSkin: - event handlers related to inputMethods not removed issues in TextFieldSkin: - memory leak due to behavior leaking - memory leaks due to manually added change/invalidation listeners that are not removed - memory leaks due to not removing children with strong references to skin - side-effects (f.i. NPEs) due to listeners/bindings that are still active after dispose Suspected issues (no way to test for me, not having the appropriate device) - there are listeners/children added only for touch/FXVK-supported systems which most probably should be removed as well. Do it here (blindly, untested) or open a follow-up issue? Fix is to cleanup all ;) Which is a bit particular in text-related skins: they rely heavily on bindings/invalidation only, so care must be taken to not validate observables accidentally (f.i. by moving them to - current - skin api). The fix will - change manual registration of changeListeners to skin api - change manual registration of invalidationListeners which effectively validate the observable (by grabbing its value along the line) to skin api - add a reference to manually added weak/invalidationListeners and manually remove them in dispose
21-12-2020

@Kevin, good point :) Looks like changing the listening on part of the skin is just cosmetics: type any char and NPE is back. The real issue might be the behavior that's not really cleaned - its focus/select/replace methods directly call those of the skin which is no longer attached to any control. Bigger can of worms than I thought ...
05-03-2020

the error is the manual registration of an InvalidationListener to a property of the control that's never removed. The fix is to either manually manage a weakInvalidationListener (if we really need to update on invalidation which I doubt) or go the self-cleaning way of using skin api: // manual registration that's never cleaned up // control.selectionProperty().addListener(observable -> { // updateSelection(); // }); // use skin api that's cleaned automatically on dispose registerChangeListener(control.selectionProperty(), o -> updateSelection());
04-03-2020

can be reproduced without jmetro, just switch the textfield to a plain subclass of TextFieldSkin (must be a subclass, another TextFieldSkin is fine) public static class MyTextFieldSkin extends TextFieldSkin { public MyTextFieldSkin(TextField control) { super(control); } } private void switchToJMetro(TextField textField) { System.out.println("Switching to JMetro"); textField.setSkin(new MyTextFieldSkin(textField)); } click the button to switch the skin, then click into the textField will throw the error shown above
04-03-2020

This might be related to JDK-8236840, which is a memory leak when switching skins.
04-03-2020