Skip to content

fix: Fix cancelling installation by not prematurely deleting patched APK #2490

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

kitadai31
Copy link
Contributor

@kitadai31 kitadai31 commented Apr 20, 2025

The issue had been caused by a race condition in asynchronous processing.
In setLastPatchedApp() method, await keyword was needed before deleteLastPatchedApp();

A code to reproduce:

Future<void> deleteLastPatchedApp() async {
  final PatchedApplication? app = getLastPatchedApp();
  if (app != null) {
    print('kitadai31: (2) Starting a 2 seconds wait');
    await Future.delayed(const Duration(seconds: 2)); // <- ADDED
    print('kitadai31: (5) 2 seconds elapsed. Delete files/lastPatchedApp.apk');
    final File file = File(app.patchedFilePath);
    await file.delete();
    await _prefs.remove('lastPatchedApp');
  }
}

Future<void> setLastPatchedApp(PatchedApplication app, File outFile) async {
  print('kitadai31: (1) calling deleteLastPatchedApp()');
  deleteLastPatchedApp();
  final Directory appCache = await getApplicationSupportDirectory();
  print('kitadai31: (3) Starting copying the patched APK to files/lastPatchedApp.apk');
  app.patchedFilePath =
      outFile.copySync('${appCache.path}/lastPatchedApp.apk').path;
  print('kitadai31: (4) Copy succeeded');
  app.fileSize = outFile.lengthSync();
  await _prefs.setString('lastPatchedApp', json.encode(app.toJson()));
}

The code is executed in the order (1) → (2) → (3) → (4) → (5).
As a result, lastPatchedApp.apk is sometimes randomly deleted after copying is succeeded.

Conclusion

The deleteLastPatchedApp() calling in setLastPatchedApp() is unnecessary from the beginning, so this PR removed this.
Because lastPatchedApp.apk and a pref will be overwritten without calling deleteLastPatchedApp().

The calling in reAssessPatchedApps() is also redundant because the file does not exist, so removed.

@kitadai31
Copy link
Contributor Author

kitadai31 commented Apr 20, 2025

#2332 (fix: Directly save APK to the cache when saving last patched app) can be reflected after this PR just to improve efficiency
but randomization should be reverted

@oSumAtrIX oSumAtrIX changed the title fix(Save patched app): Patched APK file immediately deleted and installation canceled fix: Installation cancelled due to premature deletion of patched APK Apr 25, 2025
@oSumAtrIX oSumAtrIX changed the title fix: Installation cancelled due to premature deletion of patched APK fix: Do not prematurrely delete patched APK to fix cancelling installation Apr 25, 2025
@oSumAtrIX oSumAtrIX changed the title fix: Do not prematurrely delete patched APK to fix cancelling installation fix: Fix installation cancellation by not prematurely deleting patched APK Apr 25, 2025
@oSumAtrIX oSumAtrIX changed the title fix: Fix installation cancellation by not prematurely deleting patched APK fix: Fix cancelling installation by not prematurely deleting patched APK Apr 25, 2025
Copy link

@Kimmon322 Kimmon322 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run

@kitadai31 kitadai31 requested a review from oSumAtrIX April 28, 2025 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Manager sometimes immediately cancels installation and Export APK button stops working
3 participants