Browse Source

OpenEuiccService: Fix ServiceConnection leakage

Peter Cai 1 year ago
parent
commit
083ee03f8e
1 changed files with 40 additions and 29 deletions
  1. 40 29
      app/src/main/java/im/angry/openeuicc/service/OpenEuiccService.kt

+ 40 - 29
app/src/main/java/im/angry/openeuicc/service/OpenEuiccService.kt

@@ -13,7 +13,6 @@ import im.angry.openeuicc.core.EuiccChannel
 import im.angry.openeuicc.core.EuiccChannelManager
 import im.angry.openeuicc.core.EuiccChannelManager
 import im.angry.openeuicc.util.*
 import im.angry.openeuicc.util.*
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.first
-import kotlinx.coroutines.flow.single
 import kotlinx.coroutines.runBlocking
 import kotlinx.coroutines.runBlocking
 import java.lang.IllegalStateException
 import java.lang.IllegalStateException
 
 
@@ -57,8 +56,10 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
      *
      *
      * This ensures that we only spawn and connect to APDU channels when we absolutely need to,
      * This ensures that we only spawn and connect to APDU channels when we absolutely need to,
      * instead of keeping them open unnecessarily in the background at all times.
      * instead of keeping them open unnecessarily in the background at all times.
+     *
+     * This function cannot be inline because non-local returns may bypass the unbind
      */
      */
-    private inline fun <T> withEuiccChannelManager(fn: EuiccChannelManagerContext.() -> T): T {
+    private fun <T> withEuiccChannelManager(fn: EuiccChannelManagerContext.() -> T): T {
         val (binder, unbind) = runBlocking {
         val (binder, unbind) = runBlocking {
             bindServiceSuspended(
             bindServiceSuspended(
                 Intent(
                 Intent(
@@ -168,15 +169,20 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
         Log.i(TAG, "onGetEuiccProfileInfoList slotId=$slotId")
         Log.i(TAG, "onGetEuiccProfileInfoList slotId=$slotId")
         if (slotId == -1 || shouldIgnoreSlot(slotId)) {
         if (slotId == -1 || shouldIgnoreSlot(slotId)) {
             Log.i(TAG, "ignoring slot $slotId")
             Log.i(TAG, "ignoring slot $slotId")
-            return GetEuiccProfileInfoListResult(RESULT_FIRST_USER, arrayOf(), true)
+            return@withEuiccChannelManager GetEuiccProfileInfoListResult(
+                RESULT_FIRST_USER,
+                arrayOf(),
+                true
+            )
         }
         }
 
 
         // TODO: Temporarily enable the slot to access its profiles if it is currently unmapped
         // TODO: Temporarily enable the slot to access its profiles if it is currently unmapped
-        val channel = findChannel(slotId) ?: return GetEuiccProfileInfoListResult(
-            RESULT_FIRST_USER,
-            arrayOf(),
-            true
-        )
+        val channel =
+            findChannel(slotId) ?: return@withEuiccChannelManager GetEuiccProfileInfoListResult(
+                RESULT_FIRST_USER,
+                arrayOf(),
+                true
+            )
         val profiles = channel.lpa.profiles.operational.map {
         val profiles = channel.lpa.profiles.operational.map {
             EuiccProfileInfo.Builder(it.iccid).apply {
             EuiccProfileInfo.Builder(it.iccid).apply {
                 setProfileName(it.name)
                 setProfileName(it.name)
@@ -198,7 +204,11 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
             }.build()
             }.build()
         }
         }
 
 
-        return GetEuiccProfileInfoListResult(RESULT_OK, profiles.toTypedArray(), channel.removable)
+        return@withEuiccChannelManager GetEuiccProfileInfoListResult(
+            RESULT_OK,
+            profiles.toTypedArray(),
+            channel.removable
+        )
     }
     }
 
 
     override fun onGetEuiccInfo(slotId: Int): EuiccInfo {
     override fun onGetEuiccInfo(slotId: Int): EuiccInfo {
@@ -207,30 +217,31 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
 
 
     override fun onDeleteSubscription(slotId: Int, iccid: String): Int = withEuiccChannelManager {
     override fun onDeleteSubscription(slotId: Int, iccid: String): Int = withEuiccChannelManager {
         Log.i(TAG, "onDeleteSubscription slotId=$slotId iccid=$iccid")
         Log.i(TAG, "onDeleteSubscription slotId=$slotId iccid=$iccid")
-        if (shouldIgnoreSlot(slotId)) return RESULT_FIRST_USER
+        if (shouldIgnoreSlot(slotId)) return@withEuiccChannelManager RESULT_FIRST_USER
 
 
         try {
         try {
-            val channels = findAllChannels(slotId) ?: return RESULT_FIRST_USER
+            val channels =
+                findAllChannels(slotId) ?: return@withEuiccChannelManager RESULT_FIRST_USER
 
 
             if (!channels[0].profileExists(iccid)) {
             if (!channels[0].profileExists(iccid)) {
-                return RESULT_FIRST_USER
+                return@withEuiccChannelManager RESULT_FIRST_USER
             }
             }
 
 
             // If the profile is enabled by ANY channel (port), we cannot delete it
             // If the profile is enabled by ANY channel (port), we cannot delete it
             channels.forEach { channel ->
             channels.forEach { channel ->
                 val profile = channel.lpa.profiles.find {
                 val profile = channel.lpa.profiles.find {
                     it.iccid == iccid
                     it.iccid == iccid
-                } ?: return RESULT_FIRST_USER
+                } ?: return@withEuiccChannelManager RESULT_FIRST_USER
 
 
                 if (profile.state == LocalProfileInfo.State.Enabled) {
                 if (profile.state == LocalProfileInfo.State.Enabled) {
                     // Must disable the profile first
                     // Must disable the profile first
-                    return RESULT_FIRST_USER
+                    return@withEuiccChannelManager RESULT_FIRST_USER
                 }
                 }
             }
             }
 
 
             euiccChannelManager.beginTrackedOperationBlocking(channels[0].slotId, channels[0].portId) {
             euiccChannelManager.beginTrackedOperationBlocking(channels[0].slotId, channels[0].portId) {
                 if (channels[0].lpa.deleteProfile(iccid)) {
                 if (channels[0].lpa.deleteProfile(iccid)) {
-                    return RESULT_OK
+                    return@withEuiccChannelManager RESULT_OK
                 }
                 }
 
 
                 runBlocking {
                 runBlocking {
@@ -238,9 +249,9 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
                 }
                 }
             }
             }
 
 
-            return RESULT_FIRST_USER
+            return@withEuiccChannelManager RESULT_FIRST_USER
         } catch (e: Exception) {
         } catch (e: Exception) {
-            return RESULT_FIRST_USER
+            return@withEuiccChannelManager RESULT_FIRST_USER
         }
         }
     }
     }
 
 
@@ -260,7 +271,7 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
         forceDeactivateSim: Boolean
         forceDeactivateSim: Boolean
     ): Int = withEuiccChannelManager {
     ): Int = withEuiccChannelManager {
         Log.i(TAG,"onSwitchToSubscriptionWithPort slotId=$slotId portIndex=$portIndex iccid=$iccid forceDeactivateSim=$forceDeactivateSim")
         Log.i(TAG,"onSwitchToSubscriptionWithPort slotId=$slotId portIndex=$portIndex iccid=$iccid forceDeactivateSim=$forceDeactivateSim")
-        if (shouldIgnoreSlot(slotId)) return RESULT_FIRST_USER
+        if (shouldIgnoreSlot(slotId)) return@withEuiccChannelManager RESULT_FIRST_USER
 
 
         try {
         try {
             // retryWithTimeout is needed here because this function may be called just after
             // retryWithTimeout is needed here because this function may be called just after
@@ -272,7 +283,7 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
             } ?: run {
             } ?: run {
                 if (!forceDeactivateSim) {
                 if (!forceDeactivateSim) {
                     // The user must select which SIM to deactivate
                     // The user must select which SIM to deactivate
-                    return@onSwitchToSubscriptionWithPort RESULT_MUST_DEACTIVATE_SIM
+                    return@withEuiccChannelManager RESULT_MUST_DEACTIVATE_SIM
                 } else {
                 } else {
                     try {
                     try {
                         // If we are allowed to deactivate any SIM we like, try mapping the indicated port first
                         // If we are allowed to deactivate any SIM we like, try mapping the indicated port first
@@ -282,13 +293,13 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
                         // We cannot map the port (or it is already mapped)
                         // We cannot map the port (or it is already mapped)
                         // but we can also use any port available on the card
                         // but we can also use any port available on the card
                         retryWithTimeout(5000) { findChannel(slotId) }
                         retryWithTimeout(5000) { findChannel(slotId) }
-                    } ?: return@onSwitchToSubscriptionWithPort RESULT_FIRST_USER
+                    } ?: return@withEuiccChannelManager RESULT_FIRST_USER
                 }
                 }
             }
             }
 
 
             if (iccid != null && !channel.profileExists(iccid)) {
             if (iccid != null && !channel.profileExists(iccid)) {
                 Log.i(TAG, "onSwitchToSubscriptionWithPort iccid=$iccid not found")
                 Log.i(TAG, "onSwitchToSubscriptionWithPort iccid=$iccid not found")
-                return RESULT_FIRST_USER
+                return@withEuiccChannelManager RESULT_FIRST_USER
             }
             }
 
 
             euiccChannelManager.beginTrackedOperationBlocking(channel.slotId, channel.portId) {
             euiccChannelManager.beginTrackedOperationBlocking(channel.slotId, channel.portId) {
@@ -296,11 +307,11 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
                     // Disable any active profile first if present
                     // Disable any active profile first if present
                     channel.lpa.disableActiveProfile(false)
                     channel.lpa.disableActiveProfile(false)
                     if (!channel.lpa.enableProfile(iccid)) {
                     if (!channel.lpa.enableProfile(iccid)) {
-                        return RESULT_FIRST_USER
+                        return@withEuiccChannelManager RESULT_FIRST_USER
                     }
                     }
                 } else {
                 } else {
                     if (!channel.lpa.disableActiveProfile(true)) {
                     if (!channel.lpa.disableActiveProfile(true)) {
-                        return RESULT_FIRST_USER
+                        return@withEuiccChannelManager RESULT_FIRST_USER
                     }
                     }
                 }
                 }
 
 
@@ -310,9 +321,9 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
                 }
                 }
             }
             }
 
 
-            return RESULT_OK
+            return@withEuiccChannelManager RESULT_OK
         } catch (e: Exception) {
         } catch (e: Exception) {
-            return RESULT_FIRST_USER
+            return@withEuiccChannelManager RESULT_FIRST_USER
         } finally {
         } finally {
             euiccChannelManager.invalidate()
             euiccChannelManager.invalidate()
         }
         }
@@ -324,15 +335,15 @@ class OpenEuiccService : EuiccService(), OpenEuiccContextMarker {
                 TAG,
                 TAG,
                 "onUpdateSubscriptionNickname slotId=$slotId iccid=$iccid nickname=$nickname"
                 "onUpdateSubscriptionNickname slotId=$slotId iccid=$iccid nickname=$nickname"
             )
             )
-            if (shouldIgnoreSlot(slotId)) return RESULT_FIRST_USER
-            val channel = findChannel(slotId) ?: return RESULT_FIRST_USER
+            if (shouldIgnoreSlot(slotId)) return@withEuiccChannelManager RESULT_FIRST_USER
+            val channel = findChannel(slotId) ?: return@withEuiccChannelManager RESULT_FIRST_USER
             if (!channel.profileExists(iccid)) {
             if (!channel.profileExists(iccid)) {
-                return RESULT_FIRST_USER
+                return@withEuiccChannelManager RESULT_FIRST_USER
             }
             }
             val success = channel.lpa
             val success = channel.lpa
                 .setNickname(iccid, nickname!!)
                 .setNickname(iccid, nickname!!)
             appContainer.subscriptionManager.tryRefreshCachedEuiccInfo(channel.cardId)
             appContainer.subscriptionManager.tryRefreshCachedEuiccInfo(channel.cardId)
-            return if (success) {
+            return@withEuiccChannelManager if (success) {
                 RESULT_OK
                 RESULT_OK
             } else {
             } else {
                 RESULT_FIRST_USER
                 RESULT_FIRST_USER