Skip to content

Commit f9cac7b

Browse files
adsamcikCopilot
andcommitted
fix(network): cancel in-flight OkHttp calls when kill switch is disabled (R2 round 7)
KillSwitchInterceptor only rejects NEW requests on chain entry; calls that have already passed the interceptors and are blocked on socket I/O (especially MapLibre online-tile fetches that go through the raw call factory) continued running for the full read timeout after the user toggled offline -- leaking cleartext metadata for up to 30 seconds. DefaultNetworkGateway.setEnabled(false) now calls client.dispatcher.cancelAll() on the shared OkHttp Dispatcher. Per-request clients built via newBuilder() share that Dispatcher instance, so MapLibre's tile and style requests are cancelled too. The flip is idempotent and only fires on a true enabled->disabled transition. Updated NetworkGateway.setEnabled KDoc to accurately describe both effects (reject future + cancel in-flight). Added test that enqueues a SocketPolicy.NO_RESPONSE response, launches a synchronous call on a worker thread, flips the switch off, and asserts the call is cancelled within 5s (would otherwise wait the 30s read timeout). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1335185 commit f9cac7b

3 files changed

Lines changed: 91 additions & 3 deletions

File tree

network/src/main/java/com/adsamcik/tracker/network/DefaultNetworkGateway.kt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,19 @@ class DefaultNetworkGateway(
219219
}
220220

221221
override fun setEnabled(enabled: Boolean) {
222+
val wasEnabled = _isEnabled.value
222223
_isEnabled.value = enabled
224+
// R2 round 7: when flipping the kill switch OFF, cancel every in-flight
225+
// queued + executing call on the shared OkHttp Dispatcher. Without this,
226+
// online tile fetches launched a moment before the user toggles offline
227+
// continue to completion (potentially several seconds of cleartext
228+
// metadata in flight) — the KillSwitchInterceptor only stops NEW calls.
229+
// MapLibre uses the same call factory, so its tile/style requests are
230+
// cancelled too. Per-request OkHttpClients built via newBuilder() share
231+
// this Dispatcher instance, so cancelAll() reaches them as well.
232+
if (wasEnabled && !enabled) {
233+
runCatching { client.dispatcher.cancelAll() }
234+
}
223235
}
224236

225237
override fun setPolicy(policy: NetworkPolicy) {

network/src/main/java/com/adsamcik/tracker/network/NetworkGateway.kt

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,11 +71,26 @@ interface NetworkGateway {
7171
suspend fun request(req: NetworkRequest): NetworkResponse
7272

7373
/**
74-
* Toggle the kill switch. Pass `false` to immediately reject all in-flight
75-
* and subsequent requests with [NetworkError.GatewayDisabled].
74+
* Toggle the kill switch.
75+
*
76+
* Passing `false` does two things:
77+
* 1. **Future requests** entering [request] or the raw [okhttp3.Call.Factory]
78+
* (used by MapLibre online tiles) are rejected immediately with
79+
* [NetworkError.GatewayDisabled] before any DNS / TCP / TLS work.
80+
* 2. **In-flight calls** that have already passed interceptors and are
81+
* blocked on socket I/O are actively cancelled via
82+
* `OkHttpClient.dispatcher.cancelAll()`. This prevents an online tile
83+
* fetch launched a moment before the user toggles offline from
84+
* continuing to spend cleartext metadata for the full read timeout
85+
* (interceptors only check on chain entry; they cannot interrupt a
86+
* blocking read).
87+
*
88+
* Passing `true` simply re-arms the gateway. No in-flight calls exist at
89+
* that moment (they were cancelled when the switch was flipped off, or
90+
* never started), so there is nothing to resume.
7691
*
7792
* Intended to be wired to a user-facing setting and to system events
78-
* (airplane mode, metered network, low battery at the discretion of the
93+
* (airplane mode, metered network, low battery -- at the discretion of the
7994
* consumer feature).
8095
*/
8196
fun setEnabled(enabled: Boolean)

network/src/test/java/com/adsamcik/tracker/network/DefaultNetworkGatewayTest.kt

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,67 @@ class DefaultNetworkGatewayTest {
110110
response.error shouldBe NetworkError.GatewayDisabled
111111
}
112112

113+
@Test
114+
fun `setEnabled(false) cancels in-flight OkHttp calls (R2 round 7)`() {
115+
// Before the fix, KillSwitchInterceptor only rejected NEW requests;
116+
// any call that had already passed through the application interceptors
117+
// and was waiting on socket I/O kept running for the full read timeout
118+
// (up to 30s of in-flight cleartext metadata when the user toggles
119+
// online tiles off). The fix calls client.dispatcher.cancelAll() inside
120+
// setEnabled(false) so MapLibre's pending tile fetches die immediately.
121+
val gateway = DefaultNetworkGateway(
122+
initialEnabled = true,
123+
initialPolicy = NetworkPolicy(
124+
allowedHosts = setOf("localhost"),
125+
perHostRateLimit = 1_000,
126+
perHostRateWindowMs = 60_000L,
127+
),
128+
)
129+
// Make MockWebServer accept the connection but never send any response —
130+
// the client blocks on socket read indefinitely. setBodyDelay alone is
131+
// not enough because execute() returns once headers arrive; we need the
132+
// call to still be in-flight when we toggle the kill switch.
133+
httpsServer.enqueue(
134+
MockResponse().setSocketPolicy(okhttp3.mockwebserver.SocketPolicy.NO_RESPONSE),
135+
)
136+
val client = gateway.httpsTestClient()
137+
val call = client.newCall(
138+
okhttp3.Request.Builder().url("https://localhost:${httpsServer.port}/slow").build(),
139+
)
140+
141+
// Launch the call on a background thread so we can flip the kill switch
142+
// from the test thread while it's blocked on socket read.
143+
val resultRef = java.util.concurrent.atomic.AtomicReference<Throwable?>(null)
144+
val started = java.util.concurrent.CountDownLatch(1)
145+
val finished = java.util.concurrent.CountDownLatch(1)
146+
val t = Thread {
147+
started.countDown()
148+
try {
149+
call.execute().close()
150+
} catch (e: Throwable) {
151+
resultRef.set(e)
152+
} finally {
153+
finished.countDown()
154+
}
155+
}
156+
t.start()
157+
// Wait for the worker thread to enter the blocking call.
158+
started.await(2, java.util.concurrent.TimeUnit.SECONDS)
159+
// Brief sleep so the call actually reaches the socket-read stage.
160+
Thread.sleep(200)
161+
162+
// Flip the kill switch — cancelAll() should kill the in-flight call.
163+
gateway.setEnabled(false)
164+
165+
// The call should finish quickly (cancelled), not wait the 60s delay.
166+
val cancelled = finished.await(5, java.util.concurrent.TimeUnit.SECONDS)
167+
cancelled shouldBe true
168+
call.isCanceled() shouldBe true
169+
val err = resultRef.get()
170+
err shouldNotBe null
171+
// OkHttp surfaces cancellation as IOException("Canceled") (or similar).
172+
(err is java.io.IOException) shouldBe true
173+
}
113174
@Test
114175
fun `host not in allowlist returns HostNotAllowed before any network IO`() = runTest {
115176
val gateway = DefaultNetworkGateway(

0 commit comments

Comments
 (0)