refactor: Channel validity, and reconnection
All checks were successful
/ build-debug (push) Successful in 4m51s

* ApduInterfaces also need a concept of validity based on the underlying
  APDU channel. For example, OMAPI depends on SEService being still
  connected.
* We then rely on this validity to wait for reconnection; we do not need
  to manually remove all channels under a slot because the rest will be
  invalid anyway, and the next attempt at connection will lazily
  recreate the channel.
* We had to manage channels manually before during reconnect because
  `valid` may result in SIGSEGV's when the underlying APDU channel has
  become invalid. This is avoided by the validity concept added to APDU
  channels.
This commit is contained in:
Peter Cai 2024-03-22 21:08:59 -04:00
parent 1ac683f9ab
commit e48f9aa828
8 changed files with 60 additions and 40 deletions

View file

@ -103,28 +103,35 @@ open class DefaultEuiccChannelManager(
findAllEuiccChannelsByPhysicalSlot(physicalSlotId) findAllEuiccChannelsByPhysicalSlot(physicalSlotId)
} }
override fun findEuiccChannelByPortBlocking(physicalSlotId: Int, portId: Int): EuiccChannel? = override suspend fun findEuiccChannelByPort(physicalSlotId: Int, portId: Int): EuiccChannel? =
runBlocking { withContext(Dispatchers.IO) {
withContext(Dispatchers.IO) { uiccCards.find { it.physicalSlotIndex == physicalSlotId }?.let { card ->
uiccCards.find { it.physicalSlotIndex == physicalSlotId }?.let { card -> card.ports.find { it.portIndex == portId }?.let { tryOpenEuiccChannel(it) }
card.ports.find { it.portIndex == portId }?.let { tryOpenEuiccChannel(it) }
}
} }
} }
override suspend fun tryReconnectSlot(physicalSlotId: Int, timeoutMillis: Long) { override fun findEuiccChannelByPortBlocking(physicalSlotId: Int, portId: Int): EuiccChannel? =
invalidateByPhysicalSlot(physicalSlotId) runBlocking {
findEuiccChannelByPort(physicalSlotId, portId)
}
override suspend fun waitForReconnect(physicalSlotId: Int, portId: Int, timeoutMillis: Long) {
// If there is already a valid channel, we close it proactively
// Sometimes the current channel can linger on for a bit even after it should have become invalid
channels.find { it.slotId == physicalSlotId && it.portId == portId }?.apply {
if (valid) close()
}
withTimeout(timeoutMillis) { withTimeout(timeoutMillis) {
while (true) { while (true) {
try { try {
findAllEuiccChannelsByPhysicalSlot(physicalSlotId)!!.forEach { // tryOpenEuiccChannel() will automatically dispose of invalid channels
check(it.valid) { "Invalid channel" } // and recreate when needed
} val channel = findEuiccChannelByPortBlocking(physicalSlotId, portId)!!
check(channel.valid) { "Invalid channel" }
break break
} catch (e: Exception) { } catch (e: Exception) {
Log.d(TAG, "Slot reconnect failure, retrying in 1000 ms") Log.d(TAG, "Slot $physicalSlotId port $portId reconnect failure, retrying in 1000 ms")
invalidateByPhysicalSlot(physicalSlotId)
} }
delay(1000) delay(1000)
} }
@ -157,12 +164,4 @@ open class DefaultEuiccChannelManager(
channels.clear() channels.clear()
euiccChannelFactory.cleanup() 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)
}
}
} }

View file

@ -9,13 +9,11 @@ interface EuiccChannelManager {
suspend fun enumerateEuiccChannels() suspend fun enumerateEuiccChannels()
/** /**
* Reconnect ALL EuiccChannels belonging to a physical slot * Wait for a slot + port to reconnect (i.e. become valid again)
* Throws TimeoutCancellationException when timed out * If the port is currently valid, this function will return immediately.
* If this operation times out, none of the channels belonging to the slot will be * On timeout, the caller can decide to either try again later, or alert the user with an error
* 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) suspend fun waitForReconnect(physicalSlotId: Int, portId: Int, timeoutMillis: Long = 1000)
/** /**
* Returns the EuiccChannel corresponding to a **logical** slot * Returns the EuiccChannel corresponding to a **logical** slot
@ -39,6 +37,7 @@ interface EuiccChannelManager {
/** /**
* Returns the EuiccChannel corresponding to a **physical** slot and a port ID * Returns the EuiccChannel corresponding to a **physical** slot and a port ID
*/ */
suspend fun findEuiccChannelByPort(physicalSlotId: Int, portId: Int): EuiccChannel?
fun findEuiccChannelByPortBlocking(physicalSlotId: Int, portId: Int): EuiccChannel? fun findEuiccChannelByPortBlocking(physicalSlotId: Int, portId: Int): EuiccChannel?
/** /**

View file

@ -13,6 +13,9 @@ class OmapiApduInterface(
private lateinit var session: Session private lateinit var session: Session
private lateinit var lastChannel: Channel private lateinit var lastChannel: Channel
override val valid: Boolean
get() = service.isConnected && (this::session.isInitialized && !session.isClosed)
override fun connect() { override fun connect() {
session = service.getUiccReaderCompat(port.logicalSlotIndex + 1).openSession() session = service.getUiccReaderCompat(port.logicalSlotIndex + 1).openSession()
} }

View file

@ -146,7 +146,7 @@ open class EuiccManagementFragment : Fragment(), EuiccProfilesChangedListener,
} }
try { try {
euiccChannelManager.tryReconnectSlot(slotId, timeoutMillis = 30 * 1000) euiccChannelManager.waitForReconnect(slotId, portId, timeoutMillis = 30 * 1000)
} catch (e: TimeoutCancellationException) { } catch (e: TimeoutCancellationException) {
withContext(Dispatchers.Main) { withContext(Dispatchers.Main) {
// Timed out waiting for SIM to come back online, we can no longer assume that the LPA is still valid // Timed out waiting for SIM to come back online, we can no longer assume that the LPA is still valid

View file

@ -11,6 +11,11 @@ class TelephonyManagerApduInterface(
): ApduInterface { ): ApduInterface {
private var lastChannel: Int = -1 private var lastChannel: Int = -1
override val valid: Boolean
// TelephonyManager channels will never become truly "invalid",
// just that transactions might return errors or nonsense
get() = lastChannel != -1
override fun connect() { override fun connect() {
// Do nothing // Do nothing
} }

View file

@ -9,4 +9,11 @@ interface ApduInterface {
fun logicalChannelOpen(aid: ByteArray): Int fun logicalChannelOpen(aid: ByteArray): Int
fun logicalChannelClose(handle: Int) fun logicalChannelClose(handle: Int)
fun transmit(tx: ByteArray): ByteArray fun transmit(tx: ByteArray): ByteArray
/**
* Is this APDU connection still valid?
* Note that even if this returns true, the underlying connection might be broken anyway;
* callers should further check with the LPA to fully determine the validity of a channel
*/
val valid: Boolean
} }

View file

@ -2,15 +2,6 @@ package net.typeblog.lpac_jni
interface LocalProfileAssistant { interface LocalProfileAssistant {
val valid: Boolean val valid: Boolean
get() = try {
// If we can read both eID and profiles properly, we are likely looking at
// a valid LocalProfileAssistant
eID
profiles
true
} catch (e: Exception) {
false
}
val profiles: List<LocalProfileInfo> val profiles: List<LocalProfileInfo>
val notifications: List<LocalProfileNotification> val notifications: List<LocalProfileNotification>
val eID: String val eID: String

View file

@ -11,13 +11,14 @@ import net.typeblog.lpac_jni.LocalProfileNotification
import net.typeblog.lpac_jni.ProfileDownloadCallback import net.typeblog.lpac_jni.ProfileDownloadCallback
class LocalProfileAssistantImpl( class LocalProfileAssistantImpl(
apduInterface: ApduInterface, private val apduInterface: ApduInterface,
httpInterface: HttpInterface httpInterface: HttpInterface
): LocalProfileAssistant { ): LocalProfileAssistant {
companion object { companion object {
private const val TAG = "LocalProfileAssistantImpl" private const val TAG = "LocalProfileAssistantImpl"
} }
private var finalized = false
private var contextHandle: Long = LpacJni.createContext(apduInterface, httpInterface) private var contextHandle: Long = LpacJni.createContext(apduInterface, httpInterface)
init { init {
if (LpacJni.euiccInit(contextHandle) < 0) { if (LpacJni.euiccInit(contextHandle) < 0) {
@ -28,6 +29,17 @@ class LocalProfileAssistantImpl(
httpInterface.usePublicKeyIds(pkids) httpInterface.usePublicKeyIds(pkids)
} }
override val valid: Boolean
get() = !finalized && apduInterface.valid && try {
// If we can read both eID and profiles properly, we are likely looking at
// a valid LocalProfileAssistant
eID
profiles
true
} catch (e: Exception) {
false
}
override val profiles: List<LocalProfileInfo> override val profiles: List<LocalProfileInfo>
get() = LpacJni.es10cGetProfilesInfo(contextHandle)!!.asList() get() = LpacJni.es10cGetProfilesInfo(contextHandle)!!.asList()
@ -71,8 +83,12 @@ class LocalProfileAssistantImpl(
return LpacJni.es10cSetNickname(contextHandle, iccid, nickname) == 0 return LpacJni.es10cSetNickname(contextHandle, iccid, nickname) == 0
} }
@Synchronized
override fun close() { override fun close() {
LpacJni.euiccFini(contextHandle) if (!finalized) {
LpacJni.destroyContext(contextHandle) LpacJni.euiccFini(contextHandle)
LpacJni.destroyContext(contextHandle)
finalized = true
}
} }
} }