Skip to content

Commit ebf7c4d

Browse files
gastonfourniergastonfournier
authored
fix: network status crashes (#143)
* fix: Android 11 crash when registering network listener * fix: Hardens shutdown to avoid IllegalArgumentException: NetworkCallback was already unregistered
1 parent 8be3ca2 commit ebf7c4d

File tree

2 files changed

+223
-24
lines changed

2 files changed

+223
-24
lines changed

‎unleashandroidsdk/src/main/java/io/getunleash/android/http/NetworkStatusHelper.kt‎

Lines changed: 103 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,54 +6,110 @@ import android.net.Network
66
import android.net.NetworkCapabilities
77
import android.net.NetworkRequest
88
import android.os.Build
9+
import android.os.Handler
10+
import android.os.Looper
911
import io.getunleash.android.util.UnleashLogger
12+
import java.util.concurrent.atomic.AtomicInteger
1013

1114
interface NetworkListener {
1215
fun onAvailable()
1316
fun onLost()
1417
}
1518

16-
class NetworkStatusHelper(private val context: Context) {
19+
class NetworkStatusHelper(
20+
private val context: Context,
21+
private val scheduleRetry: (Long, () -> Unit) -> Unit = { delayMs, action ->
22+
Handler(Looper.getMainLooper()).postDelayed(action, delayMs)
23+
}
24+
) {
1725
companion object {
1826
private const val TAG = "NetworkState"
27+
internal const val MAX_REGISTRATION_ATTEMPTS = 5
28+
private const val REGISTRATION_RETRY_DELAY_MS = 200L
1929
}
2030

2131
internal val networkCallbacks = mutableListOf<ConnectivityManager.NetworkCallback>()
2232

2333
private val availableNetworks = mutableSetOf<Network>()
2434

35+
private val registrationEpoch = AtomicInteger(0)
36+
2537
fun registerNetworkListener(listener: NetworkListener) {
26-
val connectivityManager = getConnectivityManager() ?: return
27-
val requestBuilder = NetworkRequest.Builder()
28-
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
29-
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
30-
requestBuilder.addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
38+
val epoch = registrationEpoch.get()
39+
registerNetworkListener(listener, MAX_REGISTRATION_ATTEMPTS, epoch)
40+
}
41+
42+
private fun registerNetworkListener(
43+
listener: NetworkListener,
44+
remainingAttempts: Int,
45+
epoch: Int
46+
) {
47+
if (epoch != registrationEpoch.get()) {
48+
UnleashLogger.d(TAG, "Skipping stale network registration attempt")
49+
return
3150
}
32-
val networkRequest = requestBuilder.build()
3351

34-
// wrap the listener in a NetworkCallback so the listener doesn't have to know about Android specifics
35-
val networkCallback = object : ConnectivityManager.NetworkCallback() {
36-
override fun onAvailable(network: Network) {
37-
availableNetworks += network
38-
listener.onAvailable()
39-
}
52+
val attemptNumber = MAX_REGISTRATION_ATTEMPTS - remainingAttempts + 1
53+
try {
54+
val connectivityManager = getConnectivityManager() ?: return
55+
val networkRequest = buildNetworkRequest()
4056

41-
override fun onLost(network: Network) {
42-
availableNetworks -= network
43-
if (availableNetworks.isEmpty()) {
44-
listener.onLost()
57+
val networkCallback = buildCallback(listener)
58+
connectivityManager.registerNetworkCallback(networkRequest, networkCallback)
59+
if (epoch != registrationEpoch.get()) {
60+
UnleashLogger.d(TAG, "Registration completed for stale attempt; unregistering callback")
61+
connectivityManager.unregisterNetworkCallback(networkCallback)
62+
return
63+
}
64+
networkCallbacks += networkCallback
65+
} catch (securityException: SecurityException) {
66+
if (remainingAttempts > 1) {
67+
UnleashLogger.w(
68+
TAG,
69+
"registerNetworkCallback failed on attempt $attemptNumber/$MAX_REGISTRATION_ATTEMPTS; retrying in $REGISTRATION_RETRY_DELAY_MS ms",
70+
securityException
71+
)
72+
if (epoch == registrationEpoch.get()) {
73+
scheduleRetry(REGISTRATION_RETRY_DELAY_MS) {
74+
registerNetworkListener(listener, remainingAttempts - 1, epoch)
75+
}
4576
}
77+
} else {
78+
UnleashLogger.w(
79+
TAG,
80+
"registerNetworkCallback failed after $attemptNumber attempts; network updates disabled",
81+
securityException
82+
)
4683
}
4784
}
48-
49-
connectivityManager.registerNetworkCallback(networkRequest, networkCallback)
50-
networkCallbacks += networkCallback
5185
}
5286

5387
fun close() {
54-
val connectivityManager = getConnectivityManager() ?: return
55-
networkCallbacks.forEach {
56-
connectivityManager.unregisterNetworkCallback(it)
88+
val epoch = registrationEpoch.incrementAndGet()
89+
val connectivityManager = getConnectivityManager() ?: run {
90+
networkCallbacks.clear()
91+
availableNetworks.clear()
92+
return
93+
}
94+
val callbacks = networkCallbacks.toList()
95+
networkCallbacks.clear()
96+
availableNetworks.clear()
97+
callbacks.forEach { callback ->
98+
try {
99+
connectivityManager.unregisterNetworkCallback(callback)
100+
} catch (illegalArgumentException: IllegalArgumentException) {
101+
UnleashLogger.w(
102+
TAG,
103+
"NetworkCallback already unregistered during close (epoch=$epoch)",
104+
illegalArgumentException
105+
)
106+
} catch (securityException: SecurityException) {
107+
UnleashLogger.w(
108+
TAG,
109+
"SecurityException while unregistering NetworkCallback during close (epoch=$epoch)",
110+
securityException
111+
)
112+
}
57113
}
58114
}
59115

@@ -93,4 +149,28 @@ class NetworkStatusHelper(private val context: Context) {
93149
fun isAvailable(): Boolean {
94150
return !isAirplaneModeOn() && isNetworkAvailable()
95151
}
152+
153+
private fun buildNetworkRequest(): NetworkRequest {
154+
val requestBuilder = NetworkRequest.Builder()
155+
.addCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET)
156+
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
157+
requestBuilder.addCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED)
158+
}
159+
return requestBuilder.build()
160+
}
161+
162+
private fun buildCallback(listener: NetworkListener) =
163+
object : ConnectivityManager.NetworkCallback() {
164+
override fun onAvailable(network: Network) {
165+
availableNetworks += network
166+
listener.onAvailable()
167+
}
168+
169+
override fun onLost(network: Network) {
170+
availableNetworks -= network
171+
if (availableNetworks.isEmpty()) {
172+
listener.onLost()
173+
}
174+
}
175+
}
96176
}

‎unleashandroidsdk/src/test/java/io/getunleash/android/http/NetworkStatusHelperTest.kt‎

Lines changed: 120 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import org.assertj.core.api.Assertions.assertThat
99
import org.junit.Test
1010
import org.mockito.ArgumentMatchers.any
1111
import org.mockito.ArgumentMatchers.anyInt
12+
import org.mockito.Mockito.doThrow
1213
import org.mockito.Mockito.mock
1314
import org.mockito.Mockito.never
1415
import org.mockito.Mockito.times
@@ -131,6 +132,124 @@ class NetworkStatusHelperTest : BaseTest() {
131132
verify(listener).onLost()
132133
}
133134

135+
@Test
136+
fun `retries registering callback when SecurityException occurs`() {
137+
val context = mock(Context::class.java)
138+
val connectivityManager = mock(ConnectivityManager::class.java)
139+
`when`(context.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(connectivityManager)
140+
doThrow(SecurityException("binder race"))
141+
.doNothing()
142+
.`when`(connectivityManager)
143+
.registerNetworkCallback(any(), any<ConnectivityManager.NetworkCallback>())
144+
145+
val scheduledActions = mutableListOf<() -> Unit>()
146+
val listener = mock(NetworkListener::class.java)
147+
148+
val networkStatusHelper = NetworkStatusHelper(
149+
context,
150+
scheduleRetry = { _, action -> scheduledActions.add(action) }
151+
)
152+
153+
networkStatusHelper.registerNetworkListener(listener)
154+
155+
assertThat(networkStatusHelper.networkCallbacks).isEmpty()
156+
assertThat(scheduledActions).hasSize(1)
157+
158+
scheduledActions.removeAt(0).invoke()
159+
160+
assertThat(networkStatusHelper.networkCallbacks).hasSize(1)
161+
verify(connectivityManager, times(2))
162+
.registerNetworkCallback(any(), any<ConnectivityManager.NetworkCallback>())
163+
}
164+
165+
@Test
166+
fun `gives up registering callback after max retries`() {
167+
val context = mock(Context::class.java)
168+
val connectivityManager = mock(ConnectivityManager::class.java)
169+
`when`(context.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(connectivityManager)
170+
doThrow(SecurityException("binder race"))
171+
.`when`(connectivityManager)
172+
.registerNetworkCallback(any(), any<ConnectivityManager.NetworkCallback>())
173+
174+
val scheduledActions = mutableListOf<() -> Unit>()
175+
val networkStatusHelper = NetworkStatusHelper(
176+
context,
177+
scheduleRetry = { _, action -> scheduledActions.add(action) }
178+
)
179+
180+
networkStatusHelper.registerNetworkListener(mock(NetworkListener::class.java))
181+
182+
while (scheduledActions.isNotEmpty()) {
183+
scheduledActions.removeAt(0).invoke()
184+
}
185+
186+
assertThat(networkStatusHelper.networkCallbacks).isEmpty()
187+
verify(connectivityManager, times(NetworkStatusHelper.MAX_REGISTRATION_ATTEMPTS))
188+
.registerNetworkCallback(any(), any<ConnectivityManager.NetworkCallback>())
189+
}
190+
191+
@Test
192+
fun `close swallows IllegalArgumentException when unregistering callback`() {
193+
val context = mock(Context::class.java)
194+
val connectivityManager = mock(ConnectivityManager::class.java)
195+
val callback = mock(ConnectivityManager.NetworkCallback::class.java)
196+
`when`(context.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(connectivityManager)
197+
doThrow(IllegalArgumentException("already unregistered"))
198+
.`when`(connectivityManager)
199+
.unregisterNetworkCallback(callback)
200+
201+
val networkStatusHelper = NetworkStatusHelper(context)
202+
networkStatusHelper.networkCallbacks += callback
203+
204+
networkStatusHelper.close()
205+
206+
verify(connectivityManager).unregisterNetworkCallback(callback)
207+
assertThat(networkStatusHelper.networkCallbacks).isEmpty()
208+
}
209+
210+
@Test
211+
fun `close does not trigger retry attempts scheduled before closing`() {
212+
val context = mock(Context::class.java)
213+
val connectivityManager = mock(ConnectivityManager::class.java)
214+
`when`(context.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(connectivityManager)
215+
doThrow(SecurityException("binder race"))
216+
.`when`(connectivityManager)
217+
.registerNetworkCallback(any(), any<ConnectivityManager.NetworkCallback>())
218+
219+
val scheduledActions = mutableListOf<() -> Unit>()
220+
val networkStatusHelper = NetworkStatusHelper(
221+
context,
222+
scheduleRetry = { _, action -> scheduledActions.add(action) }
223+
)
224+
225+
networkStatusHelper.registerNetworkListener(mock(NetworkListener::class.java))
226+
assertThat(scheduledActions).hasSize(1)
227+
228+
networkStatusHelper.close()
229+
230+
scheduledActions.removeAt(0).invoke()
231+
232+
verify(connectivityManager, times(1))
233+
.registerNetworkCallback(any(), any<ConnectivityManager.NetworkCallback>())
234+
}
235+
236+
@Test
237+
fun `close can be called multiple times without unregistering twice`() {
238+
val context = mock(Context::class.java)
239+
val connectivityManager = mock(ConnectivityManager::class.java)
240+
val callback = mock(ConnectivityManager.NetworkCallback::class.java)
241+
`when`(context.getSystemService(Context.CONNECTIVITY_SERVICE)).thenReturn(connectivityManager)
242+
243+
val networkStatusHelper = NetworkStatusHelper(context)
244+
networkStatusHelper.networkCallbacks += callback
245+
246+
networkStatusHelper.close()
247+
networkStatusHelper.close()
248+
249+
verify(connectivityManager, times(1)).unregisterNetworkCallback(callback)
250+
assertThat(networkStatusHelper.networkCallbacks).isEmpty()
251+
}
252+
134253
private fun contextWithNetwork(network: Network?, vararg capabilities: Int): Context {
135254
val context = mock(Context::class.java)
136255
val connectivityManager = mock(ConnectivityManager::class.java)
@@ -149,4 +268,4 @@ class NetworkStatusHelperTest : BaseTest() {
149268
}
150269

151270

152-
}
271+
}

0 commit comments

Comments
 (0)