From 5fc1fbc9e8dea5b26f72a4f19ae25ce3415f5f82 Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 29 Sep 2026 23:55:39 +0200 Subject: [PATCH 1/2] fix(hal): close() resolves once the serial port is actually closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Dispenser interface declared close(): void, so no caller could know when the port was free — and both drivers returned well before it was. puloon deferred serial.close() behind a 100 ms setTimeout and returned immediately. A close-then-reopen caller (setCassettes -> init) therefore raced a handle that was still open and got EAGAIN "Cannot lock port" every single time, the overlap being the full 100 ms. Worse, had the reopen ever won, the pending timer would then have closed the *new* handle and nulled the field, leaving a silently dead dispenser rather than a loud error. f56 has no timer but serialport's close() is asynchronous regardless, so it had the same race with a much narrower window — intermittent rather than deterministic, on sintra/tejo/gaia/batm3. Both now resolve on serialport's close callback, claiming the handle up front so concurrent calls can't double-close. puloon keeps its 100 ms drain (it lets an in-flight write land) but awaits it instead of firing and forgetting. --- packages/hal/src/dispensers/f56/f56-rs232.ts | 20 +++++++++-- packages/hal/src/dispensers/f56/index.ts | 8 ++--- packages/hal/src/dispensers/puloon/index.ts | 6 ++-- .../hal/src/dispensers/puloon/puloon-rs232.ts | 36 +++++++++++++++---- packages/hal/src/types.ts | 9 +++-- 5 files changed, 61 insertions(+), 18 deletions(-) diff --git a/packages/hal/src/dispensers/f56/f56-rs232.ts b/packages/hal/src/dispensers/f56/f56-rs232.ts index ae233c0..24e1d60 100644 --- a/packages/hal/src/dispensers/f56/f56-rs232.ts +++ b/packages/hal/src/dispensers/f56/f56-rs232.ts @@ -240,9 +240,25 @@ fsm.on('send', (data: Buffer) => { serial?.write(data) }) -export function close(): void { - serial?.close() +/** + * Close the serial port, resolving once the OS handle is really gone. + * + * serialport's close() is asynchronous, so returning before its callback fires + * let a close-then-reopen caller (setCassettes -> init) race the old handle and + * fail to take the exclusive lock. Narrower window than puloon's, which + * deferred the close behind a timer, but the same bug. See aiolabs/bitspire#118. + */ +export async function close(): Promise { + const port = serial + if (!port) return + // Claim the handle up front so a concurrent close() can't double-close it. serial = null + await new Promise((resolve) => { + port.close((err?: Error | null) => { + if (err) console.warn('F56 | serial close raised:', err.message) + resolve() + }) + }) } export default { diff --git a/packages/hal/src/dispensers/f56/index.ts b/packages/hal/src/dispensers/f56/index.ts index 182febf..c64882d 100644 --- a/packages/hal/src/dispensers/f56/index.ts +++ b/packages/hal/src/dispensers/f56/index.ts @@ -56,14 +56,14 @@ export class F56Dispenser implements BillDispenser { const { bills, error } = await f56.billCount(notes) if (error) { - this.close() + await this.close() ;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError' ;(error as Error & { statusCode: number }).statusCode = 570 } return { value: bills, error } } catch (err) { - this.close() + await this.close() const error = err as Error ;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError' ;(error as Error & { statusCode: number }).statusCode = 570 @@ -71,8 +71,8 @@ export class F56Dispenser implements BillDispenser { } } - close(): void { - f56.close() + async close(): Promise { + await f56.close() this.initialized = false } diff --git a/packages/hal/src/dispensers/puloon/index.ts b/packages/hal/src/dispensers/puloon/index.ts index 63186e9..f065fd4 100644 --- a/packages/hal/src/dispensers/puloon/index.ts +++ b/packages/hal/src/dispensers/puloon/index.ts @@ -50,7 +50,7 @@ export class PuloonDispenser implements BillDispenser { const { bills, error } = await this.device.dispense(notes) if (error) { - this.close() + await this.close() error.name = 'PuloonDispenseError' console.log('PULOON | dispense error', error) } @@ -58,8 +58,8 @@ export class PuloonDispenser implements BillDispenser { return { value: bills, error } } - close(): void { - this.device.close() + async close(): Promise { + await this.device.close() this.initialized = false } diff --git a/packages/hal/src/dispensers/puloon/puloon-rs232.ts b/packages/hal/src/dispensers/puloon/puloon-rs232.ts index e057b73..c579f14 100644 --- a/packages/hal/src/dispensers/puloon/puloon-rs232.ts +++ b/packages/hal/src/dispensers/puloon/puloon-rs232.ts @@ -13,6 +13,9 @@ */ import { SerialPort } from 'serialport' + +/** Grace period for an in-flight write to land before the port is closed. */ +const SERIAL_DRAIN_MS = 100 import { billLengths, encodeBillLength } from './bills.js' import type { CassetteConfig, DispenseResult } from '../../types.js' @@ -238,13 +241,32 @@ export class PuloonRs232 { } /** Close the serial port */ - close(): void { - if (this.serial) { - setTimeout(() => { - this.serial?.close() - this.serial = null - }, 100) - } + /** + * Close the serial port, resolving only once the OS handle is really gone. + * + * This used to defer `serial.close()` behind a 100 ms setTimeout and return + * immediately, with no way for a caller to know when the port was free. A + * caller that closed and reopened — setCassettes -> init — therefore raced a + * handle that was still open and got EAGAIN "Cannot lock port" every time, + * since the overlap was the full 100 ms. And if the reopen ever won the + * race, the pending timer fired afterwards and closed the *new* handle, + * leaving a silently dead dispenser. See aiolabs/bitspire#118. + * + * The drain delay is kept — it lets an in-flight write land before the port + * goes away — but it is now awaited rather than fired and forgotten. + */ + async close(): Promise { + const serial = this.serial + if (!serial) return + // Claim the handle up front so a concurrent close() can't double-close it. + this.serial = null + await new Promise((resolve) => setTimeout(resolve, SERIAL_DRAIN_MS)) + await new Promise((resolve) => { + serial.close((err?: Error | null) => { + if (err) console.warn('PULOON | serial close raised:', err.message) + resolve() + }) + }) } /** Send a command and wait for a single response */ diff --git a/packages/hal/src/types.ts b/packages/hal/src/types.ts index 0936fea..bf0615f 100644 --- a/packages/hal/src/types.ts +++ b/packages/hal/src/types.ts @@ -179,8 +179,13 @@ export interface BillDispenser { error?: Error }> - /** Close the connection */ - close(): void + /** + * Close the connection. + * + * Resolves only once the underlying port is actually closed, so a caller may + * safely reopen it straight after awaiting this (aiolabs/bitspire#118). + */ + close(): Promise /** * Check if bills are present at the dispense outlet -- 2.55.0 From 47d3d0b237ca9b3be3d57392a0355a011733c969 Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 29 Sep 2026 23:55:48 +0200 Subject: [PATCH 2/2] fix(hal-service): don't keep a cassette layout the dispenser refused setCassettes updated bays and dispenserInitData before re-initialising the device, deliberately, so that "subsequent dispense calls see the new layout even if the dispenser re-init is slow / fails". With the re-init failing every time on douro, that meant the app kept a layout the hardware had never taken, the operator-config consumer logged "Applied ops", and a cassettes-state event went out to the operator advertising it. The douro spent the afternoon reporting bay1:100x60 bay2:200x0 while the device was still running the boot-time 100x50/200x50. A dispense in that state picks bays by a layout the device does not share. Roll the in-memory layout back when the re-init throws, and let the error propagate as before. Reversing that earlier choice deliberately: a stale but honest layout beats a fresh but fictional one when the difference is which cassette pays out. Also await dispenser.close() here and in cleanup(), now that close() reports completion. Closes #118 --- apps/machine/electron/hal-service.ts | 51 ++++++++++++++++------------ 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/apps/machine/electron/hal-service.ts b/apps/machine/electron/hal-service.ts index 2ec863b..1259c77 100644 --- a/apps/machine/electron/hal-service.ts +++ b/apps/machine/electron/hal-service.ts @@ -376,12 +376,10 @@ export async function initializeHal(config: HalConfig): Promise { setCassettes: async (cassettes: CassetteConfig[]): Promise => { console.log( '[HAL] Hot-reloading cassette layout:', - cassettes - .map((c) => `bay${c.position}:${c.denomination}×${c.count ?? 0}`) - .join(', ') + cassettes.map((c) => `bay${c.position}:${c.denomination}×${c.count ?? 0}`).join(', ') ) - // Rebuild bays first so subsequent dispense calls see the new layout - // even if the dispenser re-init is slow / fails. + const previousBays = bays + const previousInitData = dispenserInitData bays = cassettes .slice() .sort((a, b) => a.position - b.position) @@ -391,31 +389,40 @@ export async function initializeHal(config: HalConfig): Promise { count: c.count ?? 0, })) dispenserInitData = { fiatCode: valConfig.fiatCode, cassettes } - // Close + re-init the dispenser so its internal per-bay state matches - // the new layout. Errors here surface to the caller (operator-config - // consumer) — the renderer can decide whether to retry. + // Close + re-init the dispenser so its internal per-bay state matches the + // new layout. close() now resolves only once the port is really closed, + // so the re-open below cannot race it (aiolabs/bitspire#118). + // + // On failure, roll the in-memory layout back. This reverses an earlier + // deliberate choice to keep the new bays "even if the dispenser re-init + // is slow / fails": with the re-init failing every time on douro, the app + // kept a layout the device had never taken and the operator-config + // consumer went on to publish a cassettes-state event advertising it. A + // subsequent dispense would then pick bays by a layout the hardware does + // not share. Better to surface the failure and stay truthful about what + // the device is actually running. try { - dispenser.close() + await dispenser.close() + await dispenser.init(dispenserInitData) } catch (err) { - console.warn('[HAL] Dispenser close during setCassettes raised:', err) + bays = previousBays + dispenserInitData = previousInitData + console.error('[HAL] Dispenser re-init failed; keeping the previous cassette layout:', err) + throw err } - await dispenser.init(dispenserInitData) console.log('[HAL] Dispenser re-initialized with new cassettes') }, cleanup: async () => { - return new Promise((resolve) => { - validator?.disable() - validator?.lightOff() - dispenser.close() - if (validator) { - validator.close((err?: Error) => { - if (err) console.error('[HAL] Validator close error:', err) - resolve() - }) - } else { + validator?.disable() + validator?.lightOff() + await dispenser.close() + if (!validator) return + await new Promise((resolve) => { + validator?.close((err?: Error) => { + if (err) console.error('[HAL] Validator close error:', err) resolve() - } + }) }) }, } -- 2.55.0