multiple value_templates in one preset make cpu go 100% and exhaust memory

[marcello@…]

What steps will reproduce the problem?

1. Add multiple value_template fields to one preset.
2. Type into fields.

What is the expected result?

The preset dialog works.

What happens instead?

CPU usage goes 100% and memory usage increases rapidly and steadily. Program gets very slow, nearly unresponsive. Must kill before it eats up all memory.

Please provide any additional information below. Attach a screenshot if possible.

Similar to #20888. Example preset attached.

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2021-07-12 02:41:41 +0200 (Mon, 12 Jul 2021)
Revision:18004
Build-Date:2021-07-12 00:42:49
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (18004 en) Linux Debian GNU/Linux 11 (bullseye)
Memory Usage: 1280 MB / 16060 MB (1054 MB allocated, but free)
Java version: 11.0.12-ea+6-post-Debian-1, Debian, OpenJDK 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.gtk.GTKLookAndFeel
Screen: :0.0 3840×2160 (scaling 1.00×1.00)
Maximum Screen Size: 3840×2160
Best cursor sizes: 16×16→16×16, 32×32→32×32
Environment variable LANG: en_US.UTF-8
System property file.encoding: UTF-8
System property sun.jnu.encoding: UTF-8
Locale info: en_US
Numbers with default locale: 1234567890 -> 1234567890
Desktop environment: X-Cinnamon
Java package: openjdk-11-jre:amd64-11.0.12+6-1
Java ATK Wrapper package: libatk-wrapper-java:all-0.38.0-2
libcommons-compress-java: libcommons-compress-java:all-1.20-1
libcommons-logging-java: libcommons-logging-java:all-1.2-2
fonts-noto: fonts-noto:all-20201225-1
liboauth-signpost-java: liboauth-signpost-java:all-1.2.1.2-3
VM arguments: [--add-modules=java.scripting,java.sql, -Djosm.restart=true, -Djava.net.useSystemProxies=true]

Plugins:
+ Mapillary (2.0.0-alpha.26-dirty)
+ apache-commons (35524)
+ apache-http (35589)
+ ejml (35458)
+ geotools (35458)
+ graphview (35640)
+ jaxb (35543)
+ jna (35662)
+ jts (35458)
+ log4j (35458)
+ opendata (35640)
+ reverter (35732)
+ routes (35543)
+ utilsplugin2 (35691)

Tagging presets:
+ ${HOME}/prj/hikemap/josm/data/defaultpresets.xml
+ ${HOME}/prj/hikemap/josm/mypresets.xml
+ ${HOME}/prj/hikemap/josm/hikingroutepreset.xml

Map paint styles:
+ ${HOME}/prj/hikemap/josm/elemstyles.mapcss
+ https://pasharm.github.io/New_basic_style_for_JOSM/New_basic_style.mapcss
+ ${HOME}/prj/hikemap/josm/hiking.mapcss
+ ${HOME}/prj/hikemap/josm/bus.mapcss
- ${HOME}/prj/hikemap/josm/mtb.mapcss
- ${HOME}/prj/hikemap/josm/piste.mapcss
- ${HOME}/prj/hikemap/josm/milestone.mapcss
+ ${HOME}/prj/hikemap/josm/my_style.mapcss
- ${HOME}/prj/hikemap/josm/bicycle.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/HiDPISupport&zip=1

[marcello@…]

Example preset that shows bug.

[Don-vip]

In 18023/josm:

fix #21117 - see #18949 - avoid infinite loop with presets with several value_template items

[marcello@…]

May I propose a simpler and more correct fix.

The root of the problems is that the test if (source != this) { is insufficient when more than one calculated field is in the dialog. The calculated fields will just fire at each other and bypass the test.

The correct fix is: Do not fire on calculated fields.

    private void setupListeners(AutoCompletingTextField textField, TaggingPresetItemGuiSupport support) {
        if (valueTemplate == null) { // only fire on normal fields
            textField.getDocument().addDocumentListener(DocumentAdapter.create(event ->
                    support.fireItemValueModified(this, key, textField.getText())));
        } else { // only listen on calculated fields
            textField.setForeground(Color.BLUE);
            support.addListener((source, key, newValue) -> {
                String valueTemplateText = valueTemplate.getText(support);
                Logging.trace("Evaluating value_template {0} for key {1} from {2} with new value {3} => {4}",
                        valueTemplate, key, source, newValue, valueTemplateText);
                textField.setItem(valueTemplateText);
            });
        }
    }

This also gets rid of the now superfluous invokeLater() .

Maybe we could fire on checkBoxes also? Just for completeness.

[Don-vip]

Sure, it's indeed a better fix!

[Don-vip]

In 18040/josm:

fix #21124 - see #21117 - better fix (patch by marcello)