fix: comprehensive stability, memory, and installer improvements#3187
fix: comprehensive stability, memory, and installer improvements#3187Anajrim01 wants to merge 15 commits intoReVanced:devfrom
Conversation
Instead of throwing on invalid values, it silently fails and filters the invalid Long out as a null
- fix Koin crash by always passing SelectedApplicationInfo.ViewModelParams when resolving SelectedAppInfoViewModel in nested destinations - serialize patcher ProgressEvent handling through a single channel consumer to prevent out-of-order/concurrent step state updates - improve PatcherWorker resilience: - rethrow UserInteractionException instead of silently swallowing it - timeout downloader loading (30s) to avoid indefinite waits - fail fast if patched APK is missing/empty before signing - delete failed output artifacts after unsuccessful runs - stop silently swallowing setForeground failures on Android 14+ - remove GlobalScope usage in uiSafe and patcher cleanup paths - make patch selection/options writes more atomic and race-safe: - DAO-level get-or-create helpers with conflict-safe inserts - transactional selection import replacement - replace several versionName!! call sites with safe fallbacks to prevent NPEs on malformed APK metadata - replace magic download cache TTL literal with a named constant This reduces startup crashes, improves patching reliability, and hardens data integrity under concurrent updates. # Conflicts: # app/src/main/java/app/revanced/manager/ui/viewmodel/AppSelectorViewModel.kt
|
I've tried to verify that all functionality remains the same without breaking anything and have come across no negative behavior except patcher slowing down a bit when GC is overwhelmed (but this is intentional to prevent it from crashing from too much memory allocation). |
| for (event in progressEventChannel) { | ||
| applyProgressEvent(event) | ||
| } |
There was a problem hiding this comment.
It consumes queued progress events and it centralizes/lifecycle-binds UI state updates and provides bounded buffering (DROP_OLDEST) to avoid unbounded memory growth during bursts (which causes UI lag and slow downs as well as contributes to OOM).
There was a problem hiding this comment.
Silently missing events due to DROP_OLDEST would be a bug by itself. There is an issue with it being spammed due to TRACE logs, but that is being fixed in another PR.
There was a problem hiding this comment.
I don't think the changes in this file make sense. The version is required for manager to work properly and know which patches are compatible.
There was a problem hiding this comment.
Would raising an error instead make more sense here? As version is required, a helpful error would provide more explaination and help than an exception or crashing when version is null or doesn't exist.
| rootInstaller.mount(packageName) | ||
| } | ||
| } | ||
| } |
| downloaderRepository.loadedDownloadersFlow.first() | ||
| downloaderRepository.loadedDownloadersFlow.first() |
There was a problem hiding this comment.
This formatting seems weird. Is this what the auto formatter is suggesting?
| for (event in progressEventChannel) { | ||
| applyProgressEvent(event) | ||
| } |
There was a problem hiding this comment.
Silently missing events due to DROP_OLDEST would be a bug by itself. There is an issue with it being spammed due to TRACE logs, but that is being fixed in another PR.
2 participants
This PR introduces a widespread set of stability and performance improvements across the application. It addresses critical crashes (including the Koin initialization failure), fixes memory exhaustion (OOM) and UI lag during heavy patching, properly supports multi-user root installations, and eliminates several data-race conditions.
Bug Fixes & Crash Resolutions
InvocationTargetExceptionoccurring on startup by properly resolving theparentBackStackEntryand passing its data.ChangelogSource.Managerfactory inUpdateViewModel..toLong()with.toLongOrNull()inBasePreferencesManagerto prevent crashes from malformed data.!!non-null assertions acrossversionNamechecks, gracefully falling back to"unknown".KeystoreManagerthat multiplied the validity duration by 24, resulting in incorrect expiry dates.Performance & Memory Improvements
Channelwith a10,000capacity and aBufferOverflow.DROP_OLDESTstrategy.Core, Concurrency, and Database
RandomtoUUID.randomUUID()inAppDatabaseto guarantee unique IDs.OptionDaoandSelectionDao'get-or-create' operations in@Transactionblocks withOnConflictStrategy.IGNOREto resolve duplicate entry exceptions.workerInputsinWorkerRepositoryto aConcurrentHashMap.GlobalScope.launchusage inPatcherViewModelandUtil.kt(uiSafe), replacing them with proper lifecycle-bound scopes and safeLooper.getMainLooper()handlers.REPLACEtoKEEPto prevent the accidental cancellation of actively running patcher/downloader jobs.Root & Installer Enhancements
--user 0. The application now correctly calculates theuserIddynamically viaProcess.myUid() / 100000.greppath matching for mounts by explicitly passing the-Fflag, ensuring file paths are evaluated as literal strings rather than regex.patchedApkis deleted if the patching process fails.Notes for Reviewers
Changelog Fix: HardcodingChangelogSource.Manageracts as a highly effective workaround for the prerelease parsing crash, bypassing the failing source argument entirely.Closes / Fixes
InvocationTargetException, upon invocation. #3153