Skip to content

Commit 0225d44

Browse files
marioortizmanerosimolus3
authored andcommitted
fix: return leased read connection to pool when rendezvous is cancelled
ReadPool's connection-lease handoff channel lacked an onUndeliveredElement handler. When a read {} (or the withAllConnections acquisition loop) was cancelled during the channel rendezvous, the leased element was silently dropped: the receiver never ran done.complete(Unit), so the owning read worker parked in done.await() forever and that connection was permanently lost from the pool. Once any lease leaked, updateSchema() (via withAllConnections -> repeat(size){ available.receive() }) could never acquire all connections and deadlocked while holding the database mutex. This is the underlying cause of #356: the reporter's app calls updateSchema on cold start while concurrent watch() queries are being cancelled (auth churn over a dead/slow network), which leaks a lease. A standalone updateSchema with no concurrent reads never leaks, which is why the minimal repro passed. Add onUndeliveredElement = { (_, done) -> done.complete(Unit) } so a dropped lease returns its connection to circulation. CompletableDeferred.complete() is idempotent, so this is safe alongside the normal finally paths. Adds ReadPoolLeaseLeakTest, which leaks a lease via concurrent cancelled reads and then asserts updateSchema completes; it fails before the fix and passes after.
1 parent d595c30 commit 0225d44

3 files changed

Lines changed: 108 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
- __Breaking change__: Aligning with Kotlin and Androidx multiplatform libraries, this
66
release removes support for `x86_64` Apple targets (`iosX64()`, `macosX64()`, `tvosX64()` and `watchosX64()`).
7+
- Fix a deadlock where a read query cancelled at the wrong moment would permanently leak a
8+
connection from the read pool, eventually causing `updateSchema()` (and other operations that
9+
acquire all connections) to hang. This is the underlying cause of #356.
710
- Fix sync status to make `syncStreams` return null when the database is initializing.
811
- Fix errors when subscribing to Sync Streams with object or array parameters.
912
- Support versions 3.6.0 of the Supabase client libraries.
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
package com.powersync
2+
3+
import co.touchlab.kermit.Logger
4+
import co.touchlab.kermit.Severity
5+
import co.touchlab.kermit.StaticConfig
6+
import co.touchlab.kermit.platformLogWriter
7+
import com.powersync.db.schema.Schema
8+
import com.powersync.test.factory
9+
import com.powersync.test.getTempDir
10+
import com.powersync.testutils.UserRow
11+
import kotlinx.coroutines.CoroutineScope
12+
import kotlinx.coroutines.Dispatchers
13+
import kotlinx.coroutines.SupervisorJob
14+
import kotlinx.coroutines.cancel
15+
import kotlinx.coroutines.joinAll
16+
import kotlinx.coroutines.launch
17+
import kotlinx.coroutines.runBlocking
18+
import kotlinx.coroutines.withTimeout
19+
import kotlinx.coroutines.withTimeoutOrNull
20+
import kotlin.test.Test
21+
import kotlin.time.Duration.Companion.milliseconds
22+
import kotlin.time.Duration.Companion.seconds
23+
24+
/**
25+
* Regression test for https://github.com/powersync-ja/powersync-kotlin/issues/356
26+
*
27+
* A `read {}` (or the `withAllConnections` acquisition loop) cancelled during the channel
28+
* rendezvous used to silently drop its connection lease: the element was handed to the channel
29+
* but the cancelled receiver never ran the `done.complete(Unit)` in its `finally`, so the owning
30+
* read worker parked in `done.await()` forever and the connection was permanently lost from the
31+
* pool. After even one such leak, `ReadPool.withAllConnections` (used by `updateSchema`) can never
32+
* acquire all connections and deadlocks while holding the database mutex.
33+
*
34+
* This must run on real dispatchers - the cancellation race does not surface under the
35+
* single-threaded virtual-time test dispatcher.
36+
*/
37+
class ReadPoolLeaseLeakTest {
38+
@Test
39+
fun updateSchemaSurvivesConcurrentReadCancellation() =
40+
runBlocking(Dispatchers.Default) {
41+
val logger =
42+
Logger(
43+
StaticConfig(
44+
minSeverity = Severity.Warn,
45+
logWriterList = listOf(platformLogWriter()),
46+
),
47+
)
48+
val scope = CoroutineScope(Dispatchers.Default + SupervisorJob())
49+
val schema = Schema(UserRow.table)
50+
51+
val db =
52+
createPowerSyncDatabaseImpl(
53+
factory = factory,
54+
schema = schema,
55+
dbFilename = "leak-test-${(0..1_000_000).random()}.db",
56+
dbDirectory = getTempDir(),
57+
logger = logger,
58+
scope = scope,
59+
)
60+
61+
try {
62+
// Force initialization so the read workers are up and offering connections.
63+
db.readLock { }
64+
65+
// Hammer the read pool with reads that are cancelled at varied points, under
66+
// contention. A read cancelled during the channel rendezvous leaks its connection
67+
// lease permanently (pre-fix), after which withAllConnections() can never get all
68+
// connections.
69+
val attackers =
70+
List(64) {
71+
scope.launch(Dispatchers.Default) {
72+
repeat(2_000) { i ->
73+
// 0ms cancels essentially immediately (most likely to land in the
74+
// handoff); 1ms/2ms spread the cancellation across other points.
75+
withTimeoutOrNull((i % 3).milliseconds) { db.readLock { } }
76+
}
77+
}
78+
}
79+
attackers.joinAll()
80+
81+
// If any lease leaked, the pool is permanently short and this never returns.
82+
// PRE-FIX: times out. POST-FIX: completes in a few ms.
83+
withTimeout(10.seconds) {
84+
db.updateSchema(schema)
85+
}
86+
} finally {
87+
// If a lease leaked, readPool.close() -> connections.joinAll() would itself block
88+
// forever on the worker parked in done.await(). Bound the cleanup so the test fails
89+
// fast on the updateSchema timeout instead of hanging; cancelling the scope tears
90+
// down any leaked worker either way.
91+
withTimeoutOrNull(5.seconds) { db.close() }
92+
scope.cancel()
93+
}
94+
}
95+
}

common/src/commonMain/kotlin/com/powersync/db/driver/ReadPool.kt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,16 @@ internal class ReadPool(
2121
size: Int = 5,
2222
private val scope: CoroutineScope,
2323
) {
24-
private val available = Channel<Pair<SQLiteConnection, CompletableDeferred<Unit>>>()
24+
private val available =
25+
Channel<Pair<SQLiteConnection, CompletableDeferred<Unit>>>(
26+
// If an element is sent but not delivered - e.g. the receiving read()/withAllConnections
27+
// coroutine was cancelled during the channel rendezvous, or the channel is closed with an
28+
// element in flight - complete its `done` so the owning worker returns the connection to
29+
// the pool instead of parking in done.await() forever. Without this, a single read
30+
// cancelled at the wrong moment permanently shrinks the pool and deadlocks any later
31+
// withAllConnections()/updateSchema().
32+
onUndeliveredElement = { (_, done) -> done.complete(Unit) },
33+
)
2534
private val connections: List<Job> =
2635
List(size) {
2736
val driver = factory()

0 commit comments

Comments
 (0)