Modify

Opened 4 weeks ago

Last modified 23 hours ago

#23802 new defect

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

Reported by: SekeRob Owned by: team
Priority: normal Milestone: 24.08
Component: Core Version:
Keywords: template_report Cc:

Description

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

Attachments (2)

23802.patch (3.1 KB ) - added by taylor.smock 4 weeks ago.
23802.debounce_ToggleDialog.patch (2.2 KB ) - added by taylor.smock 3 weeks ago.
Debounce saving ToggleDialog window geometry (don't commit)

Download all attachments as: .zip

Change History (19)

comment:1 by GerdP, 4 weeks ago

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.

comment:2 by SekeRob, 4 weeks ago

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

comment:3 by GerdP, 4 weeks ago

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.

comment:4 by SekeRob, 4 weeks ago

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.

comment:5 by skyper, 4 weeks ago

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

comment:6 by taylor.smock, 4 weeks ago

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.

comment:7 by stoecker, 4 weeks ago

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.

comment:8 by taylor.smock, 4 weeks ago

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

    IDEA additional info:
    Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
    <+>UTF-8
    diff --git a/src/org/openstreetmap/josm/data/Preferences.java b/src/org/openstreetmap/josm/data/Preferences.java
    a b  
    439439
    440440        // Backup old preferences if there are old preferences
    441441        if (initSuccessful && prefFile.exists() && prefFile.length() > 0) {
    442             Utils.copyFile(prefFile, backupFile);
     442            try {
     443                // But don't back up if the current preferences are invalid.
     444                // The validations are expensive (~2/3 CPU, ~1/3 memory), but this isn't a "hot" method
     445                PreferencesReader.validateXML(prefFile);
     446                PreferencesReader reader = new PreferencesReader(prefFile, false);
     447                reader.parse();
     448                Utils.copyFile(prefFile, backupFile);
     449            } catch (SAXException | XMLStreamException e) {
     450                Logging.trace(e);
     451                Logging.debug("Did not copy invalid preferences to " + backupFile + " due to: " + e.getMessage());
     452            }
    443453        }
    444454
    445455        try (PreferencesWriter writer = new PreferencesWriter(

comment:9 by stoecker, 4 weeks ago

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 :-)

comment:10 by stoecker, 4 weeks ago

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.

comment:11 by taylor.smock, 4 weeks ago

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.

by taylor.smock, 4 weeks ago

Attachment: 23802.patch added

comment:12 by GerdP, 3 weeks ago

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.

in reply to:  12 comment:13 by taylor.smock, 3 weeks ago

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.

by taylor.smock, 3 weeks ago

Debounce saving ToggleDialog window geometry (don't commit)

comment:14 by stoecker, 3 weeks ago

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 :-)

comment:15 by taylor.smock, 3 weeks ago

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.

comment:16 by GerdP, 3 weeks ago

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

comment:17 by taylor.smock, 23 hours ago

Milestone: 24.08
Summary: Backup preferences files get overwritten by bad preference files[PATCH] Backup preferences files get overwritten by bad preference files

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

Last edited 23 hours ago by taylor.smock (previous) (diff)

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The owner will remain team.
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from team to the specified user.
Next status will be 'needinfo'. The owner will be changed from team to SekeRob.
as duplicate The resolution will be set to duplicate. Next status will be 'closed'. The specified ticket will be cross-referenced with this ticket.
The owner will be changed from team to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.