From f854641b43d7a197036e2938f649bf8500928971 Mon Sep 17 00:00:00 2001 From: Jaap van Ekris <82339657+JaapvanEkris@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:16:21 +0200 Subject: [PATCH 1/5] Fix: Prevent memory leak from accumulating HRM event listeners This fixes issue #258 where the application crashes with a heap overflow after several hours of being stationary (no flywheel spinning). The root cause was that every time `createHrmPeripheral` was called, a new 'heartRateMeasurement' event listener was registered without removing the old one. This caused event listener accumulation over time, particularly during idle periods when the HRM watchdog timer kept resetting. The fix explicitly removes all 'heartRateMeasurement' listeners before registering a new one, preventing the accumulation of duplicate listeners that leads to the heap overflow. Fixes: #258 --- app/peripherals/PeripheralManager.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/peripherals/PeripheralManager.js b/app/peripherals/PeripheralManager.js index ca29deae15..788515112a 100644 --- a/app/peripherals/PeripheralManager.js +++ b/app/peripherals/PeripheralManager.js @@ -109,7 +109,7 @@ export function createPeripheralManager (config) { setupPeripherals() async function setupPeripherals () { - // The order is important, starting with the BLEs causes EBUSY error on the HCI socket on switching. I was not able to find the cause - its probably the order within the async initialization of the BleManager, but cannot find a proper fix + // The order is important, starting with the BLEs causes EBUSY error on the HCI socket on switching. I was not able to find the cause - its probably the order within the async initialization [...] await createAntPeripheral(config.antPlusMode) await createHrmPeripheral(config.heartRateMode) await createBlePeripheral(config.bluetoothMode) @@ -123,7 +123,7 @@ export function createPeripheralManager (config) { * @param {unknown} data for executing the command * * @see {@link https://github.com/JaapvanEkris/openrowingmonitor/blob/main/docs/Architecture.md#command-flow|The command flow documentation} - */ + */ /* eslint-disable-next-line no-unused-vars -- data is irrelevant here, but it is a standardised interface */ async function handleCommand (commandName, data) { switch (commandName) { @@ -390,6 +390,8 @@ export function createPeripheralManager (config) { } if (hrmPeripheral && hrmMode.toLocaleLowerCase() !== 'OFF'.toLocaleLowerCase()) { + // Remove any existing heartRateMeasurement listeners before adding a new one to prevent memory leaks + hrmPeripheral.removeAllListeners('heartRateMeasurement') hrmPeripheral.on('heartRateMeasurement', (heartRateMeasurement) => { // Clear the HRM watchdog as new HRM data has been received clearTimeout(hrmWatchdogTimer) From 6d703a8c9ddfc868b889148cf33fb99f84104f8f Mon Sep 17 00:00:00 2001 From: Jaap van Ekris <82339657+JaapvanEkris@users.noreply.github.com> Date: Mon, 27 Jul 2026 23:26:17 +0200 Subject: [PATCH 2/5] Fix broken comment --- app/peripherals/PeripheralManager.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/peripherals/PeripheralManager.js b/app/peripherals/PeripheralManager.js index 788515112a..a5e67c1de3 100644 --- a/app/peripherals/PeripheralManager.js +++ b/app/peripherals/PeripheralManager.js @@ -109,7 +109,8 @@ export function createPeripheralManager (config) { setupPeripherals() async function setupPeripherals () { - // The order is important, starting with the BLEs causes EBUSY error on the HCI socket on switching. I was not able to find the cause - its probably the order within the async initialization [...] + // The order is important, starting with the BLEs causes EBUSY error on the HCI socket on switching. + // @ToDo: I was not able to find the cause - its probably the order within the async initialization of the BleManager, but cannot find a proper fix await createAntPeripheral(config.antPlusMode) await createHrmPeripheral(config.heartRateMode) await createBlePeripheral(config.bluetoothMode) From 956c60e60a8e76c1cc44ed9af20d5763f91d68c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ab=C3=A1sz?= <32517724+Abasz@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:23:37 +0200 Subject: [PATCH 3/5] fix: prevent zombie HrmService instances and scanner listener leaks Add a #stopped cancellation flag and #scanReject to allow stop() to immediately abort an in-flight start() that is blocked on scanning or connecting. Ensure scanner listeners are removed in stop() so no closure can keep the HrmService reachable after destroy(). Replace direct recursive this.start() calls in error paths with a clean disconnect so only the existing once('disconnect') handler restarts scanning, eliminating duplicate concurrent start() instances. --- app/peripherals/ble/hrm/HrmService.js | 82 +++++++++++++++++++-------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/app/peripherals/ble/hrm/HrmService.js b/app/peripherals/ble/hrm/HrmService.js index 47a64b19b4..6ba1890bb0 100644 --- a/app/peripherals/ble/hrm/HrmService.js +++ b/app/peripherals/ble/hrm/HrmService.js @@ -69,17 +69,30 @@ export class HrmService extends EventEmitter { * @type {number | string | undefined} */ #serialNumber + /** + * Set to true when stop() is called so that any in-flight start() invocation + * exits cleanly and does not restart scanning. + */ + #stopped = false + /** + * Reject function for the pending scan Promise, allowing stop() to cancel + * a start() that is blocked waiting for a connectable advertisement. + * @type {((reason?: unknown) => void) | undefined} + */ + #scanReject /** * @param {import('../ble-host.interface.js').BleManager} manager */ - constructor (manager) { + constructor(manager) { super() this.#manager = manager } /* eslint-disable max-statements -- This initialises the BLE HRM handler */ - async start () { + async start() { + if (this.#stopped) { return } + this.#scanner = this.#manager.startScan({ scanFilters: [new BleManager.ServiceUUIDScanFilter(heartRateServiceUUID)] }) @@ -87,29 +100,47 @@ export class HrmService extends EventEmitter { this.#heartRateMeasurementCharacteristic?.removeAllListeners() this.#batteryLevelCharacteristic?.removeAllListeners() - const device = await new Promise((resolve) => { - /** @type {Scanner} */(this.#scanner).on('report', (eventData) => { - if (eventData.connectable) { - resolve(eventData) - } + let device + try { + device = await new Promise((resolve, reject) => { + this.#scanReject = reject + const activeScanner = /** @type {Scanner} */(this.#scanner) + activeScanner.on('report', (eventData) => { + if (eventData.connectable) { + resolve(eventData) + } + }) }) - }) + } catch { + return // stop() was called while scanning + } finally { + this.#scanReject = undefined + } log.info(`Found device (${device.parsedDataItems.localName || 'no name'})`) this.#scanner.removeAllListeners() this.#scanner.stopScan() + if (this.#stopped) { return } + this.#connection = await new Promise((/** @type {(value: Connection) => void} */resolve) => { this.#manager.connect(device.addressType, device.address, {}, (connection) => { resolve(connection) }) }) + if (this.#stopped) { + this.#connection.disconnect() + this.#connection = undefined + return + } + this.#connection.once('disconnect', () => { log.debug(`Disconnected from ${this.#connection?.peerAddress}, restart scanning`) - - this.start() + if (!this.#stopped) { + this.start() + } }) log.debug('Connected to ' + this.#connection.peerAddress) @@ -128,13 +159,15 @@ export class HrmService extends EventEmitter { }) }) - const deviceInformationService = primaryServices.find(service => service.uuid === deviceInformationServiceUUID) + if (this.#stopped) { return } + + const deviceInformationService = primaryServices.find((service) => service.uuid === deviceInformationServiceUUID) if (deviceInformationService !== undefined) { log.debug('HR device information service was discovered') const characteristics = await new Promise((/** @type {(value: { serialNumber?: GattClientCharacteristic, manufacturerId?: GattClientCharacteristic}) => void} */resolve) => { deviceInformationService.discoverCharacteristics((characteristics) => { resolve({ - serialNumber: characteristics.find(characteristic => characteristic.uuid === serialNumberUUID), manufacturerId: characteristics.find(characteristic => characteristic.uuid === manufacturerIdUUID) + serialNumber: characteristics.find((characteristic) => characteristic.uuid === serialNumberUUID), manufacturerId: characteristics.find((characteristic) => characteristic.uuid === manufacturerIdUUID) }) }) }) @@ -164,25 +197,25 @@ export class HrmService extends EventEmitter { }) } - const heartRateService = primaryServices.find(service => service.uuid === heartRateServiceUUID) + const heartRateService = primaryServices.find((service) => service.uuid === heartRateServiceUUID) if (heartRateService === undefined) { log.error(`Heart rate service not found in ${device.localName}`) - - this.start() + // Disconnect cleanly – the registered once('disconnect') handler will call this.start() + this.#connection?.disconnect() return } this.#heartRateMeasurementCharacteristic = await new Promise((resolve) => { heartRateService.discoverCharacteristics((characteristics) => { - resolve(characteristics.find(characteristic => characteristic.uuid === heartRateMeasurementUUID)) + resolve(characteristics.find((characteristic) => characteristic.uuid === heartRateMeasurementUUID)) }) }) if (this.#heartRateMeasurementCharacteristic === undefined) { log.error(`Heart rate measurement characteristic not found in ${device.localName}`) - - this.start() + // Disconnect cleanly – the registered once('disconnect') handler will call this.start() + this.#connection?.disconnect() return } @@ -193,7 +226,7 @@ export class HrmService extends EventEmitter { this.#onHeartRateNotify(value) }) - const batteryService = primaryServices.find(service => service.uuid === batteryLevelServiceUUID) + const batteryService = primaryServices.find((service) => service.uuid === batteryLevelServiceUUID) if (batteryService === undefined) { log.info(`Battery service not found in ${device.localName}`) @@ -202,7 +235,7 @@ export class HrmService extends EventEmitter { this.#batteryLevelCharacteristic = await new Promise((resolve) => { batteryService.discoverCharacteristics((characteristics) => { - resolve(characteristics.find(characteristic => characteristic.uuid === batteryLevelMeasurementUUID)) + resolve(characteristics.find((characteristic) => characteristic.uuid === batteryLevelMeasurementUUID)) }) }) @@ -228,9 +261,12 @@ export class HrmService extends EventEmitter { }) } - stop () { + stop() { + this.#stopped = true + this.#scanReject?.(new Error('HrmService stopped')) this.#batteryLevelCharacteristic?.removeAllListeners() this.#heartRateMeasurementCharacteristic?.removeAllListeners() + this.#scanner?.removeAllListeners() this.#scanner?.stopScan() return new Promise((/** @type {(value: void) => void} */resolve) => { log.debug('Shutting down HRM peripheral') @@ -249,7 +285,7 @@ export class HrmService extends EventEmitter { /** * @param {Buffer} data */ - #onHeartRateNotify (data) { + #onHeartRateNotify(data) { if (!Buffer.isBuffer(data) || data.length === 0) { log.error('Received invalid heart rate data, ignoring') @@ -305,7 +341,7 @@ export class HrmService extends EventEmitter { /** * @param {Buffer} data */ - #onBatteryNotify (data) { + #onBatteryNotify(data) { if (Buffer.isBuffer(data) && data.length > 0) { this.#batteryLevel = data.readUInt8(0) } From bc7590be839b724100695b2f555232a06b3b234d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ab=C3=A1sz?= <32517724+Abasz@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:32:59 +0200 Subject: [PATCH 4/5] fix: use once() instead of on() for CpsPeripheral disconnect handler Prevents duplicate triggerAdvertising() calls if the disconnect event is emitted more than once. Aligns with FtmsPeripheral and CscPeripheral. --- app/peripherals/ble/CpsPeripheral.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/peripherals/ble/CpsPeripheral.js b/app/peripherals/ble/CpsPeripheral.js index bcc1574ffd..9b23ea57b6 100644 --- a/app/peripherals/ble/CpsPeripheral.js +++ b/app/peripherals/ble/CpsPeripheral.js @@ -90,7 +90,7 @@ export function createCpsPeripheral (bleManager, config) { log.debug('CPS pairing request rejected') }) - _connection.on('disconnect', async () => { + _connection.once('disconnect', async () => { log.debug(`CPS client disconnected (address: ${_connection?.peerAddress}), restarting advertising`) _connection = undefined await triggerAdvertising() From 80c36ada3cddd06a9f3ab5150b368b29295c2b0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ab=C3=A1sz?= <32517724+Abasz@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:33:32 +0200 Subject: [PATCH 5/5] fix: clear Pm5RowingService broadcast timer when Pm5Peripheral is destroyed Add a stop() method to Pm5RowingService that clears #timer and call it from Pm5Peripheral.destroy(). Without this the timer's closure kept the entire Pm5RowingService reachable after the peripheral was replaced. --- app/peripherals/ble/Pm5Peripheral.js | 1 + app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/app/peripherals/ble/Pm5Peripheral.js b/app/peripherals/ble/Pm5Peripheral.js index 4e7a8b970d..e2ff5aabad 100644 --- a/app/peripherals/ble/Pm5Peripheral.js +++ b/app/peripherals/ble/Pm5Peripheral.js @@ -106,6 +106,7 @@ export function createPm5Peripheral (bleManager, config, controlCallback) { function destroy () { log.debug('Shutting down PM5 peripheral') + rowingService.stop() if (_manager !== undefined) { gattServices.forEach((service) => { diff --git a/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js b/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js index 4062ad68aa..381e413f8b 100644 --- a/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js +++ b/app/peripherals/ble/pm5/rowing-service/Pm5RowingService.js @@ -312,6 +312,10 @@ export class Pm5RowingService extends GattService { this.#genericStatusDataNotifies(this.#lastKnownMetrics, this.#previousSplitMetrics) } + stop () { + clearTimeout(this.#timer) + } + /** * @param {Metrics} metrics * @param {SplitTimeDistanceData} previousSplitMetrics