fix(hal): await the serial close, and stop reporting a cassette layout the dispenser refused #119

Merged
padreug merged 2 commits from fix/dispenser-close-race into dev 2026-09-29 21:58:32 +00:00
5 changed files with 61 additions and 18 deletions
Showing only changes of commit 5fc1fbc9e8 - Show all commits

fix(hal): close() resolves once the serial port is actually closed

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.
Padreug 2026-09-29 23:55:39 +02:00

View file

@ -240,9 +240,25 @@ fsm.on('send', (data: Buffer) => {
serial?.write(data) 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<void> {
const port = serial
if (!port) return
// Claim the handle up front so a concurrent close() can't double-close it.
serial = null serial = null
await new Promise<void>((resolve) => {
port.close((err?: Error | null) => {
if (err) console.warn('F56 | serial close raised:', err.message)
resolve()
})
})
} }
export default { export default {

View file

@ -56,14 +56,14 @@ export class F56Dispenser implements BillDispenser {
const { bills, error } = await f56.billCount(notes) const { bills, error } = await f56.billCount(notes)
if (error) { if (error) {
this.close() await this.close()
;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError' ;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError'
;(error as Error & { statusCode: number }).statusCode = 570 ;(error as Error & { statusCode: number }).statusCode = 570
} }
return { value: bills, error } return { value: bills, error }
} catch (err) { } catch (err) {
this.close() await this.close()
const error = err as Error const error = err as Error
;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError' ;(error as Error & { name: string; statusCode: number }).name = 'F56DispenseError'
;(error as Error & { statusCode: number }).statusCode = 570 ;(error as Error & { statusCode: number }).statusCode = 570
@ -71,8 +71,8 @@ export class F56Dispenser implements BillDispenser {
} }
} }
close(): void { async close(): Promise<void> {
f56.close() await f56.close()
this.initialized = false this.initialized = false
} }

View file

@ -50,7 +50,7 @@ export class PuloonDispenser implements BillDispenser {
const { bills, error } = await this.device.dispense(notes) const { bills, error } = await this.device.dispense(notes)
if (error) { if (error) {
this.close() await this.close()
error.name = 'PuloonDispenseError' error.name = 'PuloonDispenseError'
console.log('PULOON | dispense error', error) console.log('PULOON | dispense error', error)
} }
@ -58,8 +58,8 @@ export class PuloonDispenser implements BillDispenser {
return { value: bills, error } return { value: bills, error }
} }
close(): void { async close(): Promise<void> {
this.device.close() await this.device.close()
this.initialized = false this.initialized = false
} }

View file

@ -13,6 +13,9 @@
*/ */
import { SerialPort } from 'serialport' 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 { billLengths, encodeBillLength } from './bills.js'
import type { CassetteConfig, DispenseResult } from '../../types.js' import type { CassetteConfig, DispenseResult } from '../../types.js'
@ -238,13 +241,32 @@ export class PuloonRs232 {
} }
/** Close the serial port */ /** Close the serial port */
close(): void { /**
if (this.serial) { * Close the serial port, resolving only once the OS handle is really gone.
setTimeout(() => { *
this.serial?.close() * 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<void> {
const serial = this.serial
if (!serial) return
// Claim the handle up front so a concurrent close() can't double-close it.
this.serial = null this.serial = null
}, 100) await new Promise((resolve) => setTimeout(resolve, SERIAL_DRAIN_MS))
} await new Promise<void>((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 */ /** Send a command and wait for a single response */

View file

@ -179,8 +179,13 @@ export interface BillDispenser {
error?: Error 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<void>
/** /**
* Check if bills are present at the dispense outlet * Check if bills are present at the dispense outlet