fix: comprehensive stability, memory, and installer improvements by Anajrim01 · Pull Request #3187 · ReVanced/revanced-manager

Anajrim01 · 2026-03-23T18:53:13Z

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

SelectedAppInfoViewModel Crash: Fixes the Koin InvocationTargetException occurring on startup by properly resolving the parentBackStackEntry and passing its data.
Prerelease Changelog Crash: Prevents the app from crashing when fetching or updating changelogs for pre-releases by enforcing the ChangelogSource.Manager factory in UpdateViewModel.
Corrupted Preferences: Replaced .toLong() with .toLongOrNull() in BasePreferencesManager to prevent crashes from malformed data.
Nullability Sweeps: Removed dangerous !! non-null assertions across versionName checks, gracefully falling back to "unknown".
Keystore Validity: Fixed a math error in KeystoreManager that multiplied the validity duration by 24, resulting in incorrect expiry dates.

Performance & Memory Improvements

Patcher Logging UI Lag: The Patcher emits rapid log events, previously causing excessive GC thrashing, UI freezes, and OOM crashes. This is resolved by migrating the progress event emissions to a Coroutine Channel with a 10,000 capacity and a BufferOverflow.DROP_OLDEST strategy.

Core, Concurrency, and Database

Database UID Collisions: Switched from Random to UUID.randomUUID() in AppDatabase to guarantee unique IDs.
Race Conditions: Wrapped OptionDao and SelectionDao 'get-or-create' operations in @Transaction blocks with OnConflictStrategy.IGNORE to resolve duplicate entry exceptions.
Thread Safety: Converted workerInputs in WorkerRepository to a ConcurrentHashMap.
Safe Scopes: Removed dangerous GlobalScope.launch usage in PatcherViewModel and Util.kt (uiSafe), replacing them with proper lifecycle-bound scopes and safe Looper.getMainLooper() handlers.
WorkManager: Changed the enqueue policy from REPLACE to KEEP to prevent the accidental cancellation of actively running patcher/downloader jobs.

Root & Installer Enhancements

Multi-User Support: Root uninstalls/installs no longer hardcode --user 0. The application now correctly calculates the userId dynamically via Process.myUid() / 100000.
Mount Detection: Fixed grep path matching for mounts by explicitly passing the -F flag, ensuring file paths are evaluated as literal strings rather than regex.
Cleanup: Ensured that patchedApk is deleted if the patching process fails.

Notes for Reviewers

REVERTED: Changelog Fix: Hardcoding ChangelogSource.Manager acts as a highly effective workaround for the prerelease parsing crash, bypassing the failing source argument entirely.

Closes / Fixes

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

Anajrim01 · 2026-03-24T13:40:36Z

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

Axelen123 · 2026-03-30T18:22:49Z

app/src/main/java/app/revanced/manager/data/room/selection/SelectionDao.kt


-    @Insert
-    abstract suspend fun createSelection(selection: PatchSelection)
+    @Insert(onConflict = OnConflictStrategy.IGNORE)


Why would we be getting conflicts?

createSelectionIfMissing is used by getOrCreateSelectionId, which follows SELECT -> INSERT -> SELECT flow rather than a single SQL statement. With the unique (patch_bundle, package_name) constraint, concurrent callers could both observe “missing”, then one insert succeeds while the other hits a conflict (and throws).

In the current impl. this is unlikely/rare, but OnConflictStrategy.IGNORE is more of a defensive, wherre it's possible that it may happen in the future so better to avoid unexpected exceptions and handle it appropriately.

Another benefit is, its clearer to future developer that calling createSelectionIfMissing will never throw due to duplicates (from racecondtions) rather only due to other errors.

Currently, we only save selection when the user clicks the save button, so concurrency isn't a concern in this case.

Axelen123 · 2026-03-30T18:24:05Z

app/src/main/java/app/revanced/manager/domain/repository/DownloadedAppRepository.kt

+    companion object {
+        private const val CACHE_TTL_MS = 6 * 60 * 60 * 1_000L
+    }


Most files in the codebase have companion objects at the bottom of the class file.

Axelen123 · 2026-03-30T18:26:27Z

app/src/main/java/app/revanced/manager/data/room/AppDatabase.kt


    companion object {
-        fun generateUid() = Random.Default.nextInt()
+        fun generateUid() = UUID.randomUUID().mostSignificantBits.toInt()


Is there a reason for this change? Just curious

Now that I look at this, this serves no real benefit as converting to int32 just looses all the benefits of UUID uniqueness (when full) over Random.nextInt. Could be refactored to use full UUID's instead but this might break other things so skipping and reverting instead.

Axelen123 · 2026-03-30T18:30:17Z

app/src/main/java/app/revanced/manager/patcher/worker/PatcherWorker.kt

+                        withTimeout(30_000L) {
+                            downloaderRepository.loadedDownloadersFlow.first()
+                        }


This operation just retrieves the downloaders themselves. They are already loaded by the time the user starts patching and would have returned an empty list if they for some reason weren't. Why does this have a 30 minute timeout?

Aha, I had assumed it did some "loading" or initializing of the downloaders here and therefore added a timeout of 30s (not 30 minutes). But if all it does is retrieve a list, then it's indeed redundant as it'll resolve immediately anyways.

Yeah, the downloaders are downloaded at startup so its redundant.

Axelen123 · 2026-03-30T18:33:04Z

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt


        if (input.selectedApp is SelectedApp.Installed && installedApp?.installType == InstallType.MOUNT) {
-            GlobalScope.launch(Dispatchers.Main) {
+            cleanupScope.launch {


There isn't really a difference between this and just using GlobalScope, since it is not being cleaned up. Using GlobalScope is fine since the operation has a timeout anyway.

Axelen123 · 2026-03-30T18:35:13Z

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt

                    packageName,
                    input.selectedApp.version ?: withContext(Dispatchers.IO) {
-                        pm.getPackageInfo(outputFile)?.versionName!!
+                        pm.getPackageInfo(outputFile)?.versionName ?: "unknown"


Use a string resource to make the string translatable.

Axelen123 · 2026-03-30T18:36:21Z

app/src/main/java/app/revanced/manager/ui/viewmodel/PatcherViewModel.kt

+            for (event in progressEventChannel) {
+                applyProgressEvent(event)
+            }


What's the reason for this?

Axelen123 · 2026-03-30T18:45:51Z

app/src/main/java/app/revanced/manager/MainActivity.kt

+                val parentBackStackEntry = navController.navGraphEntry(it)
+                val parentData =
+                    parentBackStackEntry.getComplexArg<SelectedApplicationInfo.ViewModelParams>()
                val selectedAppInfoVm = koinViewModel<SelectedAppInfoViewModel>(
-                    viewModelStoreOwner = navController.navGraphEntry(it)
-                )
+                    viewModelStoreOwner = parentBackStackEntry
+                ) { parametersOf(parentData) }


Please extract this to a @Composable extension function. Remember to use it for SelectedApplicationInfo.Main as well.

Anajrim01 added 14 commits March 23, 2026 08:10

getPackageInfo are nullable and should be treated appropriately

125a15e

update acquire timeout

645f218

correct cert expiration timing being 192y instead of 8y

48e7bd7

fix: incorrect use of offset for ui updates

0cc6a30

fix: incorrect string interpolation leads to no versionNaming

9c0904f

Properly handle strings and get userid programmatically

4b3fa18

fix: concurrency issues may occur with multiple workers

27c5a2e

use a collision free random number gen instead

3658ff3

remove unused improts

ae787f8

fix: don't throw if long parsing fails

633ceb2

Instead of throwing on invalid values, it silently fails and filters the invalid Long out as a null

fix: update changelog source in ViewModel

2ade04e

fix: improve channel buffer handling

4d6be54

merge: sync with upstream

1fadee7

Anajrim01 marked this pull request as ready for review March 24, 2026 13:37

Ushie requested a review from Axelen123 March 29, 2026 22:57

Ushie force-pushed the dev branch from 843013f to 5f757b6 Compare March 29, 2026 23:46

Axelen123 requested changes Mar 30, 2026

View reviewed changes

Uh oh!

Conversation

Anajrim01 commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bug Fixes & Crash Resolutions

Performance & Memory Improvements

Core, Concurrency, and Database

Root & Installer Enhancements

Notes for Reviewers

Closes / Fixes

Uh oh!

Anajrim01 commented Mar 24, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Anajrim01 commented Mar 23, 2026 •

edited

Loading