NeoApplications / Neo-Backup

backup manager for android
GNU Affero General Public License v3.0
2.52k stars 124 forks source link

[Bug] Backup crashes even after manuall "kill" app clicked #760

Open MrEngineerMind opened 1 year ago

MrEngineerMind commented 1 year ago

Description Sometimes when I attempt to do a backup, I get a "BackupFailedException" error.

However, if I then exit/kill Neobackup and rerun neobackup, the backup then succeeds.

Steps To Reproduce

  1. I attempt to do a backup of a target app (with these checked: apk/data/deviceprotecteddata)
  2. I get error (see attached screenshot)
  3. I click the red triangle button inside neo to kill the app's process
  4. I try doing the backup again and it gives the same error
  5. I exit neobackup (kill it's task with a swipe up from the android task view)
  6. I rerun neobackup and attempt backup again, and this time it works without error.

So, just by exiting and rerunning neobackup made the error not happen.

Notes: 1) Outside of neobackup, I did not try to kill the target app that I was trying to backup. 2) There is a chance that when this error happens, it was when I installed a newer version of the target app after neobackup did it's startup scan. For example: I first run neobackup (it did its scan), I then did a target app update from the playstore, then I tried to use neobackup to backup that target app and I get the error. Exit neo and rerun (it does another scan) and backup works.

Screenshots Screenshot_20230409-084529

System Information(please complete the following information):

hg42 commented 1 year ago

thanks for that additional info about installing an app after the scan. It's a good point.

I remember there are similar reports from time to time mentioning this kind of error message, these could indeed fit to your description.

NB uses a broadcast handler for installs/uninstalls, meaning it tries to handle these situations.

Such package info is gathered from the package manager, when the Package object is created.

That's the relevant part of the broadcast receiver:

Package.invalidateSystemCacheForPackage(packageName)
when (intent.action.orEmpty()) {
    Intent.ACTION_PACKAGE_ADDED,
    Intent.ACTION_PACKAGE_REPLACED,
    -> {
        context.packageManager.getPackageInfo(
            packageName,
            PackageManager.GET_PERMISSIONS
        )?.let { packageInfo ->
            val appInfo = AppInfo(context, packageInfo)
            GlobalScope.launch(Dispatchers.IO) {
                db.appInfoDao.replaceInsert(appInfo)
            }
        }
    }

so it clears the file info cache for that package and gets the other package info again and then inserts it into the database.

The appinfo database should then trigger the flows (in the MainViewModel).

It may be possible, that not all relevant package objects are updated, especially if you are already inside the AppSheet, it may use a cached Package object, this shouldn't be updated via the database flows. I'm not sure if operations like enable/disable and uninstall etc. are handled inside the AppSheet.

The situation is a bit similar to the backups list attached to a package, that can change in the background while the AppSheet is open. Once it was integrated (stored) inside the Package object. That's why I removed the backupList from the Package object (actually, I changed it to be a virtual object that gets its data from a map managed globally).

I think, this virtualization would be necessary here, too.

The AppInfo we get from the package manager would be stored in a map instead of directly in the package object and the map would be changed accordingly. The Package object would get it via the packageName.

In general we should have an eye on who owns what... Something owned by another instance should not be copied into an object. Instead it should be managed outside and only referenced in the object (not directly but symbolically, e.g. via packageName).

hg42 commented 1 year ago

would be nice if you could reproduce it.

If my theories are correct, it should probably not happen, if the backup is done from the home or backup tab. I think these lists are connected to the database flows.

Not sure, if it is possible to update the package object used by AppSheet via Compose mechanisms. The list would need to be recomposed and the AppSheet in front would need to be recomposed, too.

I guess if the list is not visible (hidden by the AppSheet), it will not be recomposed at this point, only later when closing the AppSheet. I also doubt this would be the correct way, because it's part of the business logic not the UI logic.

MrEngineerMind commented 1 year ago

So if I run into this problem again, what steps do you want me to try for testing purposes?

hg42 commented 1 year ago

if you run into it again, you should create a support log after it (assuming it's already configured to be useful, maxLogLines etc.)

if you want to make it reproducible, you could try to force the situation.

E.g. according to my theory, that is:

if my theory is correct, the backup should complain.

Do the same but from backup tab:

this probably works.

Actually, I doubt that that cryptic apk directory changes, when it's only an update.

I would assume, it only changes when the app is uninstalled first, then installed again (a fresh installation should create a new cryptic code). If it happens with a simple update, it should be checked if the apk directory changes in this case.

So you could replace the step "install another version" by "uninstall, then install". Even uninstall+installing the same version again should change the apk directory code.