ExtensionInstaller: Fix mistakenly canceling unrelated waitingInstall & avoid adding duplicate entry to installing queue (#1649)

* ExtensionInstaller: Fix mistakenly canceling unrelated `waitingInstall` & avoid adding duplicate entry to installing `queue`

- Changes the installer queue from a synchronized list to a synchronized set to prevent duplicate entries.
- Updates queue removal logic to remove specific entries instead of using index-based removal.
- Refines `cancelQueue` logic to ensure the active installation is only cancelled if its download ID matches the requested ID.

* Ensure thread-safe access to the installation queue when canceling downloads
This commit is contained in:
Cuong-Tran 2026-05-19 15:44:58 +07:00 committed by GitHub
parent dc7dd8045e
commit 94eac94ce7
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 16 additions and 10 deletions

View file

@ -24,7 +24,9 @@ abstract class Installer(private val service: Service) {
private val extensionManager: ExtensionManager by injectLazy() private val extensionManager: ExtensionManager by injectLazy()
private var waitingInstall = AtomicReference<Entry?>(null) private var waitingInstall = AtomicReference<Entry?>(null)
private val queue = Collections.synchronizedList(mutableListOf<Entry>()) // KMK -->
private val queue = Collections.synchronizedSet(mutableSetOf<Entry>())
// KMK <--
private val cancelReceiver = object : BroadcastReceiver() { private val cancelReceiver = object : BroadcastReceiver() {
override fun onReceive(context: Context, intent: Intent) { override fun onReceive(context: Context, intent: Intent) {
@ -104,7 +106,9 @@ abstract class Installer(private val service: Service) {
} }
val nextEntry = queue.first() val nextEntry = queue.first()
if (waitingInstall.compareAndSet(null, nextEntry)) { if (waitingInstall.compareAndSet(null, nextEntry)) {
queue.removeAt(0) // KMK -->
queue.remove(nextEntry)
// KMK <--
processEntry(nextEntry) processEntry(nextEntry)
} }
} }
@ -129,7 +133,11 @@ abstract class Installer(private val service: Service) {
*/ */
private fun cancelQueue(downloadId: Long) { private fun cancelQueue(downloadId: Long) {
val waitingInstall = this.waitingInstall.load() val waitingInstall = this.waitingInstall.load()
val toCancel = queue.find { it.downloadId == downloadId } ?: waitingInstall ?: return // KMK -->
val toCancel = synchronized(queue) { queue.find { it.downloadId == downloadId } }
?: waitingInstall?.takeIf { it.downloadId == downloadId }
// KMK <--
?: return
if (cancelEntry(toCancel)) { if (cancelEntry(toCancel)) {
queue.remove(toCancel) queue.remove(toCancel)
if (waitingInstall == toCancel) { if (waitingInstall == toCancel) {

View file

@ -15,8 +15,7 @@ import eu.kanade.tachiyomi.extension.model.InstallStep
import eu.kanade.tachiyomi.util.lang.use import eu.kanade.tachiyomi.util.lang.use
import eu.kanade.tachiyomi.util.system.getParcelableExtraCompat import eu.kanade.tachiyomi.util.system.getParcelableExtraCompat
import eu.kanade.tachiyomi.util.system.getUriSize import eu.kanade.tachiyomi.util.system.getUriSize
import logcat.LogPriority import exh.log.xLogE
import tachiyomi.core.common.util.system.logcat
class PackageInstallerInstaller(private val service: Service) : Installer(service) { class PackageInstallerInstaller(private val service: Service) : Installer(service) {
@ -42,7 +41,7 @@ class PackageInstallerInstaller(private val service: Service) : Installer(servic
.sanitizeByFiltering(this) .sanitizeByFiltering(this)
} }
if (userAction == null) { if (userAction == null) {
logcat(LogPriority.ERROR) { "Fatal error for $intent" } xLogE("Fatal error for $intent")
continueQueue(InstallStep.Error) continueQueue(InstallStep.Error)
return return
} }
@ -95,7 +94,7 @@ class PackageInstallerInstaller(private val service: Service) : Installer(servic
session.commit(intentSender) session.commit(intentSender)
} }
} catch (e: Exception) { } catch (e: Exception) {
logcat(LogPriority.ERROR, e) { "Failed to install extension ${entry.downloadId} ${entry.uri}" } xLogE("Failed to install extension ${entry.downloadId} ${entry.uri}", e)
activeSession?.let { (_, sessionId) -> activeSession?.let { (_, sessionId) ->
packageInstaller.abandonSession(sessionId) packageInstaller.abandonSession(sessionId)
} }

View file

@ -16,9 +16,8 @@ import eu.kanade.tachiyomi.extension.installer.ShizukuInstaller
import eu.kanade.tachiyomi.extension.util.ExtensionInstaller.Companion.EXTRA_DOWNLOAD_ID import eu.kanade.tachiyomi.extension.util.ExtensionInstaller.Companion.EXTRA_DOWNLOAD_ID
import eu.kanade.tachiyomi.util.system.getSerializableExtraCompat import eu.kanade.tachiyomi.util.system.getSerializableExtraCompat
import eu.kanade.tachiyomi.util.system.notificationBuilder import eu.kanade.tachiyomi.util.system.notificationBuilder
import logcat.LogPriority import exh.log.xLogE
import tachiyomi.core.common.i18n.stringResource import tachiyomi.core.common.i18n.stringResource
import tachiyomi.core.common.util.system.logcat
import tachiyomi.i18n.MR import tachiyomi.i18n.MR
class ExtensionInstallService : Service() { class ExtensionInstallService : Service() {
@ -53,7 +52,7 @@ class ExtensionInstallService : Service() {
BasePreferences.ExtensionInstaller.PACKAGEINSTALLER -> PackageInstallerInstaller(this) BasePreferences.ExtensionInstaller.PACKAGEINSTALLER -> PackageInstallerInstaller(this)
BasePreferences.ExtensionInstaller.SHIZUKU -> ShizukuInstaller(this) BasePreferences.ExtensionInstaller.SHIZUKU -> ShizukuInstaller(this)
else -> { else -> {
logcat(LogPriority.ERROR) { "Not implemented for installer $installerUsed" } xLogE("Not implemented for installer $installerUsed")
stopSelf() stopSelf()
return START_NOT_STICKY return START_NOT_STICKY
} }