fix(bluetooth): stop leaking 20Hz input poll tasks (#229) - #230
Conversation
UIManager:scheduleIn does not return a handle in KOReader, so assigning its result to poll_task left stopPolling unable to unschedule the callback. Each reconnect started another 50ms loop. Track the callback function itself (as DbusMonitor already does) and always clean up scheduled Bluetooth work on suspend even if the radio is already off. Co-authored-by: Kevin <OGKevin@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (4)**/*.lua📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
spec/**/*_spec.lua📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{lua,sh,md}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🔇 Additional comments (5)
WalkthroughThe Bluetooth polling loop now stores its callback instead of a scheduler handle. It prevents duplicate starts and avoids rescheduling after Sequence Diagram(s)sequenceDiagram
participant BluetoothKeybindings
participant UIManager
participant IsolatedReaders
BluetoothKeybindings->>UIManager: scheduleIn polling callback
UIManager->>BluetoothKeybindings: invoke callback
BluetoothKeybindings->>IsolatedReaders: poll readers
BluetoothKeybindings->>UIManager: reschedule while polling is active
BluetoothKeybindings->>UIManager: unschedule callback when polling stops
Poem
Merge Risk: ⚪ Minimal · up to This PR fixes Bluetooth polling-task cleanup and suspend handling. No actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
Triage of #229: the report has strong merit. The crashlog analysis is right about the KOReader scheduler API, and that bug can keep the UI loop waking at 20 Hz (and multiply those loops on reconnect).
What #229 claimed
BluetoothKeyBindings:startPolling()storesself.poll_task = UIManager:scheduleIn(...).UIManager:scheduleInreturns nothing, sopoll_taskstaysnil.stopPolling()cannot unschedule the callback, so loops leak across reconnect / re-init.onSuspend()only tears down Bluetooth when the radio is still reported on, so leftover tasks can keep running.KOReader validation
From frontend/ui/uimanager.lua:
scheduleIn()callsschedule()and does not return a value.unschedule(action)removes queue entries by function identity (_task_queue[i].action == action).tickAfterNext()is the API that does return a wrapper specifically so callers can unschedule it.poll_interval = 0.05) task prevents long idle / standby while KOReader is awake.DbusMonitoralready used the correct pattern (store the callback, unschedule that function). The test mock ofscheduleInreturned a numeric id, which is why unit tests never caught this.Fix
scheduleIn/unschedule._cleanup()on suspend, even if the radio is already off.scheduleInand the suspend cleanup path.Test plan
busted spec/bluetooth_keybindings_spec.lua spec/kobo_bluetooth_spec.lualuacheck/styluaon touched filesbusted spec/in CIFixes #229