[PATCH] Backup preferences files get overwritten by bad preference files

[SekeRob]

We suffer here frequent power network overloads resulting in spikes / glitches, causing the main fuse to pop. When restarting the comp and JOSM often a message appears saying that the preferences are damaged, a backup generated and new clean preferences files created, as if JOSM is in day 1 state. When then trying to put the backup preferences.xml and default_preferences.xml back in, JOSM says these are broken and the cycle is repeated. Come this experience I've been doing manual backups of the 2 files found in

C:\Users\...\AppData\Local\JOSM\cache and
C:\Users\...\AppData\Roaming\JOSM

After putting these manual backup in place, JOSM starts fine with all my settings/styles/presets/plugins etc to include the CS comments made up to the point of the manual backup.

Do not really understand why the recovery procedure is overwriting the last backup, rather I'd expect that if preferences.osm/default_preferences.osm is/are considered bad, the first action would be to overwrite those with the assumed to be autogenerated good backups. At any rate I'd ask to review the procedure and restore the last 'good' backup rather than overwrite during recovery which seemingly is happening.

The thought has crossed my mind that the manual restore of the 2 pref files never fails so maybe there never is a good JOSM generated backup in the first place. E.g. there are momentarily after the recovery a preferences.xml.bak dated the 12th and a preferences.xml.backup timestamp around when last manually recovering. Of default_preferences there only exists a .backup. From past discussion on the community forum (discourse) it seems I'm not the only one doing manual/automated backup of the prefs i.e. I'm not alone. Maybe this could be written up in a wiki page to explain which is which and what action to take to get the good prefs back.

thanks

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

Relative:URL: ^/trunk
Repository:UUID: 0c6e7542-c601-0410-84e7-c038aed88b3b
Last:Changed Date: 2024-07-02 17:10:50 +0200 (Tue, 02 Jul 2024)
Revision:19128
Build-Date:2024-07-03 01:31:15
URL:https://josm.openstreetmap.de/svn/trunk

Identification: JOSM/1.5 (19128 en) Windows 10 64-Bit
OS Build number: Windows 10 Home 22H2 (19045)
Memory Usage: 1168 MB / 4084 MB (270 MB allocated, but free)
Java version: 21.0.3+9-LTS, Azul Systems, Inc., OpenJDK 64-Bit Server VM
Look and Feel: com.sun.java.swing.plaf.windows.WindowsLookAndFeel
Screen: \Display0 1920x1080x32bpp@60Hz (scaling 1.00×1.00)
Maximum Screen Size: 1920×1080
Best cursor sizes: 16×16→32×32, 32×32→32×32
System property file.encoding: UTF-8
System property sun.jnu.encoding: Cp1252
Locale info: en_US
Numbers with default locale: 1234567890 -> 1234567890
VM arguments: [-Dicedtea-web.bin.location=%UserProfile%\AppData\Local\Programs\OpenWebStart\javaws, -Djava.util.Arrays.useLegacyMergeSort=true, --add-reads=java.base=ALL-UNNAMED,java.desktop, --add-reads=java.desktop=ALL-UNNAMED,java.naming, --add-reads=java.naming=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/sun.awt=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/javax.jnlp=ALL-UNNAMED,java.desktop, --add-exports=java.base/com.sun.net.ssl.internal.ssl=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.net.www.protocol.jar=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.action=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.provider=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.util=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.validator=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.security.x509=ALL-UNNAMED,java.desktop, --add-exports=java.base/jdk.internal.util.jar=ALL-UNNAMED,java.desktop, --add-exports=java.base/sun.net.www.protocol.http=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/sun.awt.X11=ALL-UNNAMED,java.desktop, --add-exports=java.desktop/sun.applet=ALL-UNNAMED,java.desktop,jdk.jsobject, --add-exports=java.naming/com.sun.jndi.toolkit.url=ALL-UNNAMED,java.desktop, --add-modules=java.scripting,java.sql,javafx.controls,javafx.media,javafx.swing,javafx.web, --add-exports=java.desktop/com.apple.eawt=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.spi=ALL-UNNAMED, --add-exports=java.desktop/com.sun.imageio.plugins.jpeg=ALL-UNNAMED, --add-exports=javafx.graphics/com.sun.javafx.application=ALL-UNNAMED, --add-exports=jdk.deploy/com.sun.deploy.config=ALL-UNNAMED, --add-opens=java.base/java.lang=ALL-UNNAMED, --add-opens=java.base/java.nio=ALL-UNNAMED, --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED, --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED, --add-opens=java.desktop/javax.imageio.spi=ALL-UNNAMED, --add-opens=java.desktop/javax.swing.text.html=ALL-UNNAMED, --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED, -Djava.security.manager=allow]

Plugins:
+ HouseNumberTaggingTool (36226)
+ KartaView (490)
+ Mapillary (1769)
+ OpeningHoursEditor (36258)
+ RoadSigns (36258)
+ apache-commons (36273)
+ buildings_tools (36226)
+ easypresets (1623509627)
+ graphview (36258)
+ gridify (1718663815)
+ libphonenumber (8.13.41)
+ measurement (36256)
+ notesolver (0.5.0)
+ phonenumber (1.1.1)
+ pt_assistant (637)
+ reltoolbox (36280)
+ reverter (36256)
+ routing (36226)
+ scripting (v0.3.1)
+ tageditor (36258)
+ terracer (36205)
+ todo (137)
+ turnlanes (36206)
+ turnlanes-tagging (0.0.5)
+ turnrestrictions (36226)
+ utilsplugin2 (36241)

Tagging presets:
+ <josm.pref>/EasyPresets.xml
+ https://josm.openstreetmap.de/josmfile?page=Presets/OneClick&zip=1
+ https://raw.githubusercontent.com/osmlab/name-suggestion-index/main/dist/presets/nsi-josm-presets.min.xml
+ https://josm.openstreetmap.de/josmfile?page=Presets/Golf_Course&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/AdvertisingPreset&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/Maxspeed-zones&zip=1
+ https://raw.githubusercontent.com/yopaseopor/traffic_signs_preset_JOSM/master/IT.zip
+ https://josm.openstreetmap.de/josmfile?page=Presets/Manholes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/LaneAttributes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Presets/TurnLanes&zip=1
+ https://github.com/kendzi/Simple3dBuildingsPreset/releases/download/0.9_2018-05-08/s3db-preset.zip

Map paint styles:
+ https://josm.openstreetmap.de/josmfile?page=Styles/Potlach2_access&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Admin_Boundaries&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/AddressValidator&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransport&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Lane_and_Road_Attributes&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/PublicTransportV2&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/ColorWays&zip=1
- https://josm.openstreetmap.de/josmfile?page=Rules/IncompleteObjectWarnings&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/Coloured_buildings_en&zip=1
- https://josm.openstreetmap.de/josmfile?page=Styles/Enhanced_Lane_and_Road_Attributes&zip=1
+ https://gitlab.com/cartocite/josm-style-traffic-signs-orientation/-/raw/main/traffic_sign_orientation_style.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/Modified&zip=1
+ https://josm.openstreetmap.de/josmfile?page=Styles/MapWithAI&zip=1
+ https://raw.githubusercontent.com/yopaseopor/traffic_signs_style_JOSM/master/Styles_Traffic_signs_EUR_OC.zip
- https://josm.openstreetmap.de/josmfile?page=Styles/Maxspeed&zip=1
- https://raw.githubusercontent.com/OpenNauticalChart/josm/master/european-waterways-classification-style/CEMT-style.mapcss
- https://josm.openstreetmap.de/josmfile?page=Styles/Waterways&zip=1

Last errors/warnings:
- 00029.936 E: Failed to locate image 'traffic_signs_presets/ES_road_beacon_big.png'
- 00029.937 W:  Reset Dummy (00): Could not get presets icon traffic_signs_presets/ES_road_beacon_big.png
- 00029.975 E: Failed to locate image 'traffic_signs/IT/IT_II.50-110.png'
- 00029.976 W:  Maxspeed 50 (67): Could not get presets icon traffic_signs/IT/IT_II.50-110.png
- 00029.979 E: Failed to locate image 'traffic_signs_presets/ES_r101.png'
- 00029.980 W:  No Entry Sign (70): Could not get presets icon traffic_signs_presets/ES_r101.png
- 00029.981 E: Failed to locate image 'traffic_signs/IT/IT_II.76.png'
- 00029.982 W:  Disabled parking space (71): Could not get presets icon traffic_signs/IT/IT_II.76.png
- 00029.988 E: Failed to locate image 'traffic_signs/IT/IT_II.305.png'
- 00029.989 W:  Emergency Bay node (80): Could not get presets icon traffic_signs/IT/IT_II.305.png

[GerdP]

I agree that the backup is rarely of any use.
The preferences file is overwritten very often. Each change will also overwrite the backup.
I think when JOSM says that it could not read the preferences.xml it already tried the backup as well.

[SekeRob]

As a supplemental point, I make these manual backups when JOSM is closed.

[GerdP]

Yes, anything else would be dangerous, esp. when you often see power failures. Anyway, I think every user is responsible for a reasonable backup strategy.

[SekeRob]

Seen it somewhere that the backup will not be overwritten, rather the last backup is renamed, then the original gets backed up as normal and only then the previous backup is deleted, i.e. ensures the backup process is complete before deleting the old-renamed backup. Think you'll have a much better chance of recovering the config near loss-less.

[skyper]

Similar to #22376.
#20672 would be a solution.

[taylor.smock]

The current preference save strategy is as follows:

1. Copy current preferences file to preferences.xml_backup
2. Write current preferences to preferences.xml_tmp
3. Move preferences.xml_tmp to preferences.xml

What we should do is validate that the current preferences file is valid before copying it to the backup file.

In other words,

1. Validate the current preferences file; if valid copy current preferences file to preferences.xml_backup
2. Write current preferences to preferences.xml_tmp
3. Move preferences.xml_tmp to preferences.xml

For full details, see Preferences.java@19141:432-457#L432​.

I'll want to do a bit of profiling to see if this has a significant performance impact.

[stoecker]

Hmm.

2.5) Validate the written preferences.xml_tmp file (at least check the size)?

There must be a reason that "1) Validate the current preferences file" is necessary.

[taylor.smock]

If you look at the current code, it does a "basic" check of the current preferences file; specifically, if (initSuccessful && prefFile.exists() && prefFile.length() > 0).

I'm thinking there is something wrong with our error recovery, but I haven't gotten around to checking that.

Anyway, based off of my profiling, I don't expect there to be a significant performance hit if we perform a more complete validation of the current preferences file prior to copying the current preferences file to the backup preferences file.

src/org/openstreetmap/josm/data/Preferences.java

@@ -439 +439 @@
 
         // Backup old preferences if there are old preferences
         if (initSuccessful && prefFile.exists() && prefFile.length() > 0) {
-            Utils.copyFile(prefFile, backupFile);
+            try {
+                // But don't back up if the current preferences are invalid.
+                // The validations are expensive (~2/3 CPU, ~1/3 memory), but this isn't a "hot" method
+                PreferencesReader.validateXML(prefFile);
+                PreferencesReader reader = new PreferencesReader(prefFile, false);
+                reader.parse();
+                Utils.copyFile(prefFile, backupFile);
+            } catch (SAXException | XMLStreamException e) {
+                Logging.trace(e);
+                Logging.debug("Did not copy invalid preferences to " + backupFile + " due to: " + e.getMessage());
+            }
         }
 
         try (PreferencesWriter writer = new PreferencesWriter(

[stoecker]

My guess from the reports over the years is that
a) The initial load works
b) Our "lots of pref changes" cause a write failure (disk full or whatever causes this - unknown)
c) This propagates
d) Next restart fails.

But that's only a guess. It happens seldom and never was conclusive, so please do research with a fresh approach. We never found the reason :-)

[stoecker]

P.S. I'd add a similar block after the newly written file. Or normally I'd write the file first in memory and write it as one to disk (that would be my Perl approach). Then I only need to check the size and also it's less likely to fail.

[taylor.smock]

Probably write to disk then check would be better -- that would let us catch issues where what was written is not what was expected. I don't think what we have in memory is ever the issue anyway.

[GerdP]

I'll try the patch in the next days.
My current understanding is that we sometimes update the preferences files multiple times within milliseconds, e.g. when window geometries are updated? No idea how the different OS file systems handle this in combination with a power loss.

[taylor.smock]

Replying to GerdP:

My current understanding is that we sometimes update the preferences files multiple times within milliseconds, e.g. when window geometries are updated?

We probably shouldn't be doing that. If it becomes a major problem, we can debounce the problem paths using a timer or scheduled executor. We'll see what appears during "normal" usage -- I wasn't able to visibly notice GUI issues while testing anyway (by dragging a toggle dialog in circles).

No idea how the different OS file systems handle this in combination with a power loss.

The way the files are copied should be safe w.r.t. power loss. In the event of power loss, at least one file will be valid. Realistically, we should be able to remove the validity check for preferences.xml when copying to preferences.xml_backup since we are checking the validity when writing preferences.xml. I'm inclined to keep that validity check for now just in case a user upgrades from an "old" JOSM version with a bad preferences.xml file that then gets copied over the good preferences.xml_backup file for some reason.

[taylor.smock]

Debounce saving ToggleDialog window geometry (don't commit)

[stoecker]

That's a side effect of evolving the preferences system over the time :-( Delaying the storage of less important prefs can help. Thought I would put such functionality into the Preferences class instead.

Something like "putMinor(...)" which then delays the disk-storage until there are

• major changes saved
• no changes for e.g. 5s
• 15s since the first change / last saved change

or something along these lines.

That way putMinor() can be used for any changes which could cause such effects. It's also better for encapsulation (i.e. in case we every switch to a SQL database :-)

[taylor.smock]

I like that idea better than what I did. attachment:23802.debounce_ToggleDialog.patch​ was mostly to demonstrate that we could delay preference saving if we really wanted/needed to.

[GerdP]

Feedback for 23802.patch: I didn't notice any delay while mapping +1500 objects. I didn't test it further.

[taylor.smock]

Note to self: only apply attachment:23802.patch​.

[taylor.smock]

In 19179/josm:

Fix #23802: Backup preferences files get overwritten by bad preference files

This validates a preferences file before copying/moving it to another file.
This should avoid cases where a file is partially written and then copied or
moved over a "good" preferences file.