From 5fc1fbc9e8dea5b26f72a4f19ae25ce3415f5f82 Mon Sep 17 00:00:00 2001 From: Padreug Date: Tue, 29 Sep 2026 23:55:39 +0200 Subject: [PATCH] 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