From 1ac683f9aba9182f63d379f33b4ae40bf8f0bc9d Mon Sep 17 00:00:00 2001 From: Peter Cai Date: Thu, 21 Mar 2024 22:21:24 -0400 Subject: [PATCH] refactor: Reconnecting channels is a EuiccChannelManager responsibility Reconnecting did not work properly for OMAPI, because in that case we have to reconnect SEService as well. --- .../core/DefaultEuiccChannelFactory.kt | 2 +- .../core/DefaultEuiccChannelManager.kt | 45 +++++++++-- .../openeuicc/core/EuiccChannelManager.kt | 10 +++ .../openeuicc/ui/EuiccManagementFragment.kt | 74 ++++++++++--------- .../UnprivilegedOpenEuiccApplication.kt | 3 +- .../lpac_jni/LocalProfileAssistant.kt | 4 +- .../impl/LocalProfileAssistantImpl.kt | 67 ++--------------- 7 files changed, 98 insertions(+), 107 deletions(-) diff --git a/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelFactory.kt b/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelFactory.kt index c741f49..1cb3650 100644 --- a/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelFactory.kt +++ b/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelFactory.kt @@ -10,7 +10,7 @@ open class DefaultEuiccChannelFactory(protected val context: Context) : EuiccCha private var seService: SEService? = null private suspend fun ensureSEService() { - if (seService == null) { + if (seService == null || !seService!!.isConnected) { seService = connectSEService(context) } } diff --git a/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt b/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt index a978677..ec676d1 100644 --- a/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt +++ b/app-common/src/main/java/im/angry/openeuicc/core/DefaultEuiccChannelManager.kt @@ -6,10 +6,12 @@ import android.util.Log import im.angry.openeuicc.di.AppContainer import im.angry.openeuicc.util.* import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.delay import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout open class DefaultEuiccChannelManager( protected val appContainer: AppContainer, @@ -87,14 +89,18 @@ open class DefaultEuiccChannelManager( } } + override suspend fun findAllEuiccChannelsByPhysicalSlot(physicalSlotId: Int): List? { + for (card in uiccCards) { + if (card.physicalSlotIndex != physicalSlotId) continue + return card.ports.mapNotNull { tryOpenEuiccChannel(it) } + .ifEmpty { null } + } + return null + } + override fun findAllEuiccChannelsByPhysicalSlotBlocking(physicalSlotId: Int): List? = runBlocking { - for (card in uiccCards) { - if (card.physicalSlotIndex != physicalSlotId) continue - return@runBlocking card.ports.mapNotNull { tryOpenEuiccChannel(it) } - .ifEmpty { null } - } - return@runBlocking null + findAllEuiccChannelsByPhysicalSlot(physicalSlotId) } override fun findEuiccChannelByPortBlocking(physicalSlotId: Int, portId: Int): EuiccChannel? = @@ -106,6 +112,25 @@ open class DefaultEuiccChannelManager( } } + override suspend fun tryReconnectSlot(physicalSlotId: Int, timeoutMillis: Long) { + invalidateByPhysicalSlot(physicalSlotId) + + withTimeout(timeoutMillis) { + while (true) { + try { + findAllEuiccChannelsByPhysicalSlot(physicalSlotId)!!.forEach { + check(it.valid) { "Invalid channel" } + } + break + } catch (e: Exception) { + Log.d(TAG, "Slot reconnect failure, retrying in 1000 ms") + invalidateByPhysicalSlot(physicalSlotId) + } + delay(1000) + } + } + } + override suspend fun enumerateEuiccChannels() { withContext(Dispatchers.IO) { for (uiccInfo in uiccCards) { @@ -132,4 +157,12 @@ open class DefaultEuiccChannelManager( channels.clear() euiccChannelFactory.cleanup() } + + private fun invalidateByPhysicalSlot(physicalSlotId: Int) { + val toRemove = channels.filter { it.valid && it.slotId == physicalSlotId } + for (channel in toRemove) { + channel.close() + channels.remove(channel) + } + } } \ No newline at end of file diff --git a/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt b/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt index b0bce1d..5ee40f2 100644 --- a/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt +++ b/app-common/src/main/java/im/angry/openeuicc/core/EuiccChannelManager.kt @@ -8,6 +8,15 @@ interface EuiccChannelManager { */ suspend fun enumerateEuiccChannels() + /** + * Reconnect ALL EuiccChannels belonging to a physical slot + * Throws TimeoutCancellationException when timed out + * If this operation times out, none of the channels belonging to the slot will be + * guaranteed to be consistent. The caller should either call invalidate() + * and try again later, or the application should simply exit entirely. + */ + suspend fun tryReconnectSlot(physicalSlotId: Int, timeoutMillis: Long = 1000) + /** * Returns the EuiccChannel corresponding to a **logical** slot */ @@ -24,6 +33,7 @@ interface EuiccChannelManager { * Returns all EuiccChannels corresponding to a **physical** slot * Multiple channels are possible in the case of MEP */ + suspend fun findAllEuiccChannelsByPhysicalSlot(physicalSlotId: Int): List? fun findAllEuiccChannelsByPhysicalSlotBlocking(physicalSlotId: Int): List? /** diff --git a/app-common/src/main/java/im/angry/openeuicc/ui/EuiccManagementFragment.kt b/app-common/src/main/java/im/angry/openeuicc/ui/EuiccManagementFragment.kt index 98b3d3a..c02f7fd 100644 --- a/app-common/src/main/java/im/angry/openeuicc/ui/EuiccManagementFragment.kt +++ b/app-common/src/main/java/im/angry/openeuicc/ui/EuiccManagementFragment.kt @@ -31,7 +31,6 @@ import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.withContext -import java.lang.Exception open class EuiccManagementFragment : Fragment(), EuiccProfilesChangedListener, EuiccChannelFragmentMarker { @@ -132,48 +131,51 @@ open class EuiccManagementFragment : Fragment(), EuiccProfilesChangedListener, fab.isEnabled = false lifecycleScope.launch { - try { - if (enable) { - doEnableProfile(iccid) + beginTrackedOperation { + val res = if (enable) { + channel.lpa.enableProfile(iccid) } else { - doDisableProfile(iccid) + channel.lpa.disableProfile(iccid) } - refresh() - fab.isEnabled = true - } catch (e: TimeoutCancellationException) { - // Timed out waiting for SIM to come back online, we can no longer assume that the LPA is still valid - AlertDialog.Builder(requireContext()).apply { - setMessage(R.string.enable_disable_timeout) - setPositiveButton(android.R.string.ok) { dialog, _ -> - dialog.dismiss() - requireActivity().finish() - } - setOnDismissListener { _ -> - requireActivity().finish() - } - show() + + if (!res) { + Log.d(TAG, "Failed to enable / disable profile $iccid") + Toast.makeText(context, R.string.toast_profile_enable_failed, Toast.LENGTH_LONG) + .show() + return@beginTrackedOperation false + } + + try { + euiccChannelManager.tryReconnectSlot(slotId, timeoutMillis = 30 * 1000) + } catch (e: TimeoutCancellationException) { + withContext(Dispatchers.Main) { + // Timed out waiting for SIM to come back online, we can no longer assume that the LPA is still valid + AlertDialog.Builder(requireContext()).apply { + setMessage(R.string.enable_disable_timeout) + setPositiveButton(android.R.string.ok) { dialog, _ -> + dialog.dismiss() + requireActivity().finish() + } + setOnDismissListener { _ -> + requireActivity().finish() + } + show() + } + } + return@beginTrackedOperation false + } + + if (enable) { + preferenceRepository.notificationEnableFlow.first() + } else { + preferenceRepository.notificationDisableFlow.first() } - } catch (e: Exception) { - Log.d(TAG, "Failed to enable / disable profile $iccid") - Log.d(TAG, Log.getStackTraceString(e)) - fab.isEnabled = true - Toast.makeText(context, R.string.toast_profile_enable_failed, Toast.LENGTH_LONG).show() } + refresh() + fab.isEnabled = true } } - private suspend fun doEnableProfile(iccid: String) = - beginTrackedOperation { - channel.lpa.enableProfile(iccid, reconnectTimeout = 15 * 1000) && - preferenceRepository.notificationEnableFlow.first() - } - - private suspend fun doDisableProfile(iccid: String) = - beginTrackedOperation { - channel.lpa.disableProfile(iccid, reconnectTimeout = 15 * 1000) && - preferenceRepository.notificationDisableFlow.first() - } - protected open fun populatePopupWithProfileActions(popup: PopupMenu, profile: LocalProfileInfo) { popup.inflate(R.menu.profile_options) if (profile.isEnabled) { diff --git a/app-unpriv/src/main/java/im/angry/openeuicc/UnprivilegedOpenEuiccApplication.kt b/app-unpriv/src/main/java/im/angry/openeuicc/UnprivilegedOpenEuiccApplication.kt index 5541501..7993b50 100644 --- a/app-unpriv/src/main/java/im/angry/openeuicc/UnprivilegedOpenEuiccApplication.kt +++ b/app-unpriv/src/main/java/im/angry/openeuicc/UnprivilegedOpenEuiccApplication.kt @@ -15,7 +15,8 @@ class UnprivilegedOpenEuiccApplication : OpenEuiccApplication() { override fun onCreate() { super.onCreate() - Thread.setDefaultUncaughtExceptionHandler { _, _ -> + Thread.setDefaultUncaughtExceptionHandler { _, e -> + e.printStackTrace() Intent(this, LogsActivity::class.java).apply { addFlags(Intent.FLAG_ACTIVITY_CLEAR_TASK) addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) diff --git a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/LocalProfileAssistant.kt b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/LocalProfileAssistant.kt index fbc853d..118a963 100644 --- a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/LocalProfileAssistant.kt +++ b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/LocalProfileAssistant.kt @@ -19,8 +19,8 @@ interface LocalProfileAssistant { // All blocking functions in this class assume that they are executed on non-Main threads // The IO context in Kotlin's coroutine library is recommended. - fun enableProfile(iccid: String, reconnectTimeout: Long = 0): Boolean - fun disableProfile(iccid: String, reconnectTimeout: Long = 0): Boolean + fun enableProfile(iccid: String): Boolean + fun disableProfile(iccid: String): Boolean fun deleteProfile(iccid: String): Boolean fun downloadProfile(smdp: String, matchingId: String?, imei: String?, diff --git a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt index 246ead2..98a3969 100644 --- a/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt +++ b/libs/lpac-jni/src/main/java/net/typeblog/lpac_jni/impl/LocalProfileAssistantImpl.kt @@ -1,9 +1,6 @@ package net.typeblog.lpac_jni.impl import android.util.Log -import kotlinx.coroutines.delay -import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.withTimeout import net.typeblog.lpac_jni.LpacJni import net.typeblog.lpac_jni.ApduInterface import net.typeblog.lpac_jni.EuiccInfo2 @@ -14,8 +11,8 @@ import net.typeblog.lpac_jni.LocalProfileNotification import net.typeblog.lpac_jni.ProfileDownloadCallback class LocalProfileAssistantImpl( - private val apduInterface: ApduInterface, - private val httpInterface: HttpInterface + apduInterface: ApduInterface, + httpInterface: HttpInterface ): LocalProfileAssistant { companion object { private const val TAG = "LocalProfileAssistantImpl" @@ -31,48 +28,6 @@ class LocalProfileAssistantImpl( httpInterface.usePublicKeyIds(pkids) } - private fun tryReconnect(timeoutMillis: Long) = runBlocking { - withTimeout(timeoutMillis) { - try { - LpacJni.euiccFini(contextHandle) - LpacJni.destroyContext(contextHandle) - contextHandle = -1 - } catch (e: Exception) { - // Ignored - } - - while (true) { - try { - apduInterface.disconnect() - } catch (e: Exception) { - // Ignored - } - - try { - apduInterface.connect() - contextHandle = LpacJni.createContext(apduInterface, httpInterface) - check(LpacJni.euiccInit(contextHandle) >= 0) { "Reconnect attempt failed" } - // Validate that we can actually use the APDU channel by trying to read eID and profiles - check(valid) { "Reconnected channel is invalid" } - break - } catch (e: Exception) { - e.printStackTrace() - if (contextHandle != -1L) { - try { - LpacJni.euiccFini(contextHandle) - LpacJni.destroyContext(contextHandle) - contextHandle = -1 - } catch (e: Exception) { - // Ignored - } - } - // continue retrying - delay(1000) - } - } - } - } - override val profiles: List get() = LpacJni.es10cGetProfilesInfo(contextHandle)!!.asList() @@ -87,21 +42,11 @@ class LocalProfileAssistantImpl( override val euiccInfo2: EuiccInfo2? get() = LpacJni.es10cexGetEuiccInfo2(contextHandle) - override fun enableProfile(iccid: String, reconnectTimeout: Long): Boolean { - val res = LpacJni.es10cEnableProfile(contextHandle, iccid) == 0 - if (reconnectTimeout > 0) { - tryReconnect(reconnectTimeout) - } - return res - } + override fun enableProfile(iccid: String): Boolean = + LpacJni.es10cEnableProfile(contextHandle, iccid) == 0 - override fun disableProfile(iccid: String, reconnectTimeout: Long): Boolean { - val res = LpacJni.es10cDisableProfile(contextHandle, iccid) == 0 - if (reconnectTimeout > 0) { - tryReconnect(reconnectTimeout) - } - return res - } + override fun disableProfile(iccid: String): Boolean = + LpacJni.es10cDisableProfile(contextHandle, iccid) == 0 override fun deleteProfile(iccid: String): Boolean { return LpacJni.es10cDeleteProfile(contextHandle, iccid) == 0