fix: eliminate BLE keepalive coroutine accumulation (COLUMBA-40)#530
Merged
torlando-tech merged 1 commit intomainfrom Feb 23, 2026
Merged
fix: eliminate BLE keepalive coroutine accumulation (COLUMBA-40)#530torlando-tech merged 1 commit intomainfrom
torlando-tech merged 1 commit intomainfrom
Conversation
Contributor
Greptile SummaryFixes OOM by eliminating unbounded coroutine accumulation in BLE keepalive mechanisms. Converted
The fix addresses COLUMBA-40 where 512MB heap exhaustion occurred on Samsung Galaxy Z Fold 5 under sustained BLE connection churn. All call sites verified to be in suspend contexts. Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 9ca6185 |
|
|
||
| // Continue with regular interval | ||
| while (isActive) { | ||
| delay(BleConstants.CONNECTION_KEEPALIVE_INTERVAL_MS) |
Contributor
There was a problem hiding this comment.
missing early-exit guard after delay (like client keepalive at BleGattClient.kt:1146-1154) to prevent keepalive from continuing after disconnect
Suggested change
| delay(BleConstants.CONNECTION_KEEPALIVE_INTERVAL_MS) | |
| delay(BleConstants.CONNECTION_KEEPALIVE_INTERVAL_MS) | |
| // Early exit if connection was removed by another code path | |
| val stillConnected = centralsMutex.withLock { | |
| connectedCentrals.containsKey(address) | |
| } | |
| if (!stillConnected) { | |
| Log.d(TAG, "Keepalive stopping for $address: connection no longer exists") | |
| return@launch | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: reticulum/src/main/java/com/lxmf/messenger/reticulum/ble/server/BleGattServer.kt
Line: 1024
Comment:
missing early-exit guard after delay (like client keepalive at BleGattClient.kt:1146-1154) to prevent keepalive from continuing after disconnect
```suggestion
delay(BleConstants.CONNECTION_KEEPALIVE_INTERVAL_MS)
// Early exit if connection was removed by another code path
val stillConnected = centralsMutex.withLock {
connectedCentrals.containsKey(address)
}
if (!stillConnected) {
Log.d(TAG, "Keepalive stopping for $address: connection no longer exists")
return@launch
}
```
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
… OOM Convert startKeepalive and startPeripheralKeepalive from fire-and-forget scope.launch wrappers to suspend functions, since all call sites are already in suspend contexts. Replace the untracked disconnect coroutine with an inline call (nulling keepaliveJob first to prevent self- cancellation). Add connection-exists guard after delay() to stop keepalives for already-disconnected devices. Fixes COLUMBA-40: OutOfMemoryError from coroutine accumulation under sustained BLE churn on Samsung Galaxy Z Fold 5. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e5a7979 to
9ca6185
Compare
Owner
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
startKeepalive(client) andstartPeripheralKeepalive(server) from fire-and-forgetscope.launchwrappers tosuspendfunctions — all call sites are already in suspend contextsscope.launch { disconnect(address) }with inlinedisconnect()call, nullingkeepaliveJobfirst to prevent self-cancellationdelay()to stop keepalives for already-disconnected devicesFixes COLUMBA-40:
OutOfMemoryErrorat 512MB heap from unbounded coroutine accumulation under sustained BLE churn (Samsung Galaxy Z Fold 5, Android 16).Test plan
./gradlew :reticulum:compileDebugKotlin— builds clean./gradlew :reticulum:testDebugUnitTest --tests "*.BleGattClient*"— all tests pass🤖 Generated with Claude Code