Fix #25010 – Deleted favorites reappear after app restart#25262
Closed
nazar-kutz wants to merge 11 commits into
Closed
Fix #25010 – Deleted favorites reappear after app restart#25262nazar-kutz wants to merge 11 commits into
nazar-kutz wants to merge 11 commits into
Conversation
Member
|
The tombstone journal is a reasonable short-term mitigation for deletes, but it does not address the root problem: every favourite edit still triggers a full GPX read/merge/rewrite/backup pipeline, so persistence remains slow and killable. It also only protects deletes, while adds/edits can still be lost before the async save completes. |
This patch introduces a coalescing mechanism for rapid favorite save requests. By wrapping parameters in `SaveFavoritesParams`, newer save requests now safely merge with and cancel pending tasks, preventing unnecessary disk I/O queueing. Key improvements: - Added `taskLock` in `FavouritesFileHelper` for thread-safe task merging and cancellation. - Fixed `ConcurrentModificationException` during group iteration. - Added `isCancelled()` checks to abort superseded tasks early. - Guaranteed data integrity by properly merging groups and accumulating listeners.
- Replaced individual file write operations with a single batch write mechanism (`savePendingDeletions`) when deleting multiple favorite points or groups. - Improved disk performance by accumulating serialized deletion entries in memory and writing them to the temporary file in a single `BufferedWriter` transaction. - Updated `FavouritesHelper` to pass collections directly instead of iterating and triggering separate I/O calls for each deleted item.
- delete redundant code - restore significant comments
- Replace internal file diffing with tombstone to detect deleted points - Write external files first as source of truth, internal snapshot after - Clear tombstone only after the last queued task completes - Rename pendingSaveTask to runningSaveTask
# Conflicts: # OsmAnd/src/net/osmand/plus/myplaces/favorites/FavouritesFileHelper.java # OsmAnd/src/net/osmand/plus/myplaces/favorites/FavouritesHelper.java # OsmAnd/src/net/osmand/plus/myplaces/favorites/SaveFavoritesTask.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Deleting a favorite point and immediately closing the app caused
the point to reappear after restart. This happened because deletion
was applied to in-memory state immediately, but the actual GPX file
save was performed asynchronously. If the process was killed before
the async task completed, the GPX files still contained the deleted
points. On next launch, loadFavorites() would merge them back from
the external files.
The issue was especially noticeable with large favorites collections
(10,000+ points) where the save task could take 30+ seconds.
Root cause
saveCurrentPointsIntoFile(true) queues an AsyncTask on a
singleThreadExecutor. Android can kill the process before this task
completes, leaving GPX files in the old state.
Fix
Introduced a tombstone mechanism via PendingFavoriteDeletions —
a lightweight append-only file (favorites_deletions.tmp) that is
written synchronously at the moment of deletion, before the async
save is queued.
On the next loadFavorites() call, pending deletions are applied
to both internal and external group maps before merge, ensuring
deleted points and groups are filtered out regardless of whether
the previous async save completed.
The tombstone file is cleared inside SaveFavoritesTask.saveAllGroups()
only after all GPX writes succeed, so if a save fails mid-way the
tombstone is preserved for the next launch.
Changes
point keys and group names, handles serialization
write tombstone in delete() and deleteFavourite()
Testing
Reproduced on OsmAnd+ 5.3.8 / 5.4.0 with 23,000+ favorites.
Verified fix by deleting a favorite and force-stopping the app
immediately — point no longer reappears after restart.