fix: harden container lifecycle and gRPC after cross-review

- stop() keeps the container handle until removal succeeds, tolerates 304/404,
  and shares one in-flight operation between concurrent callers.
- start() removes the container on any failure after it was created,
  including the last port-conflict attempt and startup timeouts.
- startGRPCServer resolves with the bound port, shuts down a previous
  server first, and every gRPC failure rejects instead of throwing.
- Test teardown only sweeps containers created by the current run.
- Bounded overrides: ws ^8.21.0 and uuid ^11.1.1 (uuid 14 is ESM-only).
- test:watch builds first; gRPC tests exercise Exchange end to end;
  relaxed fail-fast timing bounds.

Author: jleni

package.json

@@ -25,7 +25,7 @@
     "build": "rm -rf dist && tsc && cp src/grpc/*.proto dist/grpc/",
     "test:clean": "pnpm ts-node tests/pullImageKillOld.ts",
     "test": "pnpm test:clean && pnpm build && vitest run",
-    "test:watch": "vitest",
+    "test:watch": "pnpm build && vitest",
     "lint": "biome lint src/ tests/",
     "lint:fix": "biome check --write --unsafe .",
     "format": "biome format --write src/ tests/",
@@ -69,8 +69,8 @@
   "pnpm": {
     "overrides": {
       "axios": "^1.15.1",
-      "ws": ">=8.21.0",
-      "uuid": ">=11.1.1"
+      "ws": "^8.21.0",
+      "uuid": "^11.1.1"
     }
   }
 }

pnpm-lock.yaml

@@ -77,7 +77,7 @@ export default class Zemu {
   private readonly desiredSpeculosApiPort?: number
 
   private readonly emuContainer: EmuContainer
-  public readonly containerName: string
+  public containerName: string
   private lastTransportError: Error | null = null
 
   public readonly elfPath: string
@@ -128,14 +128,17 @@ export default class Zemu {
     await new Promise<void>((resolve) => setTimeout(resolve, timeInMs))
   }
 
-  /** Force-removes every zemu container, giving up after KILL_TIMEOUT. */
-  static async stopAllEmuContainers(): Promise<void> {
+  /**
+   * Force-removes every zemu container, giving up after KILL_TIMEOUT.
+   * Pass `createdAfter` (unix seconds) to leave containers of other sessions alone.
+   */
+  static async stopAllEmuContainers(createdAfter?: number): Promise<void> {
     let timer: NodeJS.Timeout | undefined
     const timeout = new Promise<never>((_, reject) => {
       timer = setTimeout(() => reject(new Error(`Could not kill all containers within ${KILL_TIMEOUT}ms`)), KILL_TIMEOUT)
     })
     try {
-      await Promise.race([EmuContainer.killContainerByName(BASE_NAME), timeout])
+      await Promise.race([EmuContainer.killContainerByName(BASE_NAME, createdAfter), timeout])
     } finally {
       clearTimeout(timer)
     }
@@ -189,20 +192,17 @@ export default class Zemu {
     this.log('Checking ELF')
     Zemu.checkElf(this.startOptions.model, this.elfPath)
 
-    try {
-      await this.runContainerWithFreePorts()
+    await this.runContainerWithFreePorts()
 
+    // From here on a running container exists: never leave it behind on failure
+    try {
       this.log('Connecting to container')
-      await this.connect().catch(async (error) => {
-        this.log(`${error}`)
-        await this.close()
-        throw error
-      })
-
+      await this.connect()
       await this.finalizeStart()
-    } catch (e) {
-      this.log(`[ZEMU] ${e}`)
-      throw e
+    } catch (error) {
+      this.log(`[ZEMU] ${error}`)
+      await this.close().catch((closeError) => this.log(`[ZEMU] Cleanup after failed start: ${closeError}`))
+      throw error
     }
   }
 
@@ -296,12 +296,15 @@ export default class Zemu {
         })
         return
       } catch (error) {
+        this.log(`[ZEMU] ${error}`)
+        // Docker may have created the container without starting it; drop it so the
+        // name is free again and nothing is left behind
+        await this.emuContainer.stop().catch((stopError) => this.log(`[ZEMU] Cleanup after failed container start: ${stopError}`))
+
         const portConflict = /port is already allocated|address already in use/i.test(String(error))
         if (!portConflict || attempt >= MAX_ATTEMPTS) throw error
 
-        this.log(`Port conflict, retrying with new ports: ${error}`)
-        // Docker created the container but could not start it; drop it before retrying
-        await this.emuContainer.stop().catch((stopError) => this.log(`Cleanup after port conflict failed: ${stopError}`))
+        this.log('Port conflict, retrying with new ports')
         this.transportPort = undefined as unknown as number
         this.speculosApiPort = undefined as unknown as number
       }
@@ -315,10 +318,17 @@ export default class Zemu {
     }
   }
 
-  /** Resolves once the gRPC server is listening. Rejects if the address cannot be bound. */
-  startGRPCServer(ip: string, port: number): Promise<void> {
-    this.grpcManager = new GRPCRouter(ip, port, this.transport)
-    return this.grpcManager.startServer()
+  /**
+   * Starts a gRPC server that forwards Exchange calls to the device transport.
+   * Resolves with the bound port (useful when `port` is 0). Rejects if the address cannot be bound.
+   * A previously started server is shut down first.
+   */
+  async startGRPCServer(ip: string, port: number): Promise<number> {
+    this.stopGRPCServer()
+    const router = new GRPCRouter(ip, port, this.transport)
+    const boundPort = await router.startServer()
+    this.grpcManager = router
+    return boundPort
   }
 
   stopGRPCServer(): void {

src/Zemu.ts

@@ -41,6 +41,7 @@ export const DEFAULT_START_OPTIONS: IStartOptions = {
   startDelay: DEFAULT_START_DELAY,
   custom: '',
   model: DEFAULT_MODEL,
+  sdk: '',
   startText: '',
   caseSensitive: false,
   startTimeout: DEFAULT_START_TIMEOUT,

src/constants.ts

@@ -40,6 +40,7 @@ export default class EmuContainer {
   private readonly image: string
   private readonly libElfs: Record<string, string>
   private currentContainer?: Container
+  private stopping?: Promise<void>
 
   constructor(elfLocalPath: string, libElfs: Record<string, string>, image: string, name: string) {
     this.image = image
@@ -55,11 +56,15 @@ export default class EmuContainer {
     }
   }
 
-  /** Force-removes every container whose name contains `name`. */
-  static async killContainerByName(name: string): Promise<void> {
+  /**
+   * Force-removes every container whose name contains `name`.
+   * `createdAfter` (unix seconds) limits the sweep to containers created after that time.
+   */
+  static async killContainerByName(name: string, createdAfter?: number): Promise<void> {
     const docker = new Docker()
     const containers = await docker.listContainers({ all: true, filters: { name: [name] } })
-    await Promise.all(containers.map((info) => docker.getContainer(info.Id).remove({ force: true })))
+    const targets = createdAfter === undefined ? containers : containers.filter((info) => info.Created >= createdAfter)
+    await Promise.all(targets.map((info) => docker.getContainer(info.Id).remove({ force: true })))
   }
 
   static async checkAndPullImage(imageName: string): Promise<void> {
@@ -187,28 +192,44 @@ export default class EmuContainer {
     this.log(`[ZEMU] Started ${this.currentContainer.id}`)
   }
 
-  async stop(): Promise<void> {
-    if (this.currentContainer == null) return
+  /**
+   * Stops and removes the container. The handle is only dropped once the
+   * container is gone, so a failed attempt can be retried with another stop().
+   * Concurrent calls share the same in-flight operation.
+   */
+  stop(): Promise<void> {
+    if (this.stopping == null) {
+      this.stopping = this.doStop().finally(() => {
+        this.stopping = undefined
+      })
+    }
+    return this.stopping
+  }
 
+  private async doStop(): Promise<void> {
     const container = this.currentContainer
-    this.currentContainer = undefined
+    if (container == null) return
+
     this.log('[ZEMU] Stopping container')
     try {
       await container.stop({ t: 0 })
     } catch (e: any) {
-      // 304: already stopped. Anything else is a real failure.
-      if (e?.statusCode !== 304) {
+      // 304: already stopped, 404: already gone. Anything else is a real failure.
+      if (e?.statusCode !== 304 && e?.statusCode !== 404) {
         this.log(`[ZEMU] Stopping: ${e}`)
         throw e
       }
     }
     this.log('[ZEMU] Stopped')
     try {
-      await container.remove()
-    } catch (err) {
-      this.log('[ZEMU] Unable to remove container')
-      throw err
+      await container.remove({ force: true })
+    } catch (e: any) {
+      if (e?.statusCode !== 404) {
+        this.log(`[ZEMU] Unable to remove container: ${e}`)
+        throw e
+      }
     }
+    this.currentContainer = undefined
     this.log('[ZEMU] Removed')
   }
 }

src/emulator.ts

@@ -18,10 +18,10 @@ export default class GRPCRouter {
     this.server = new Server()
   }
 
-  /** Resolves once the server is bound and listening. */
-  startServer(): Promise<void> {
+  /** Resolves with the bound port once the server is listening. */
+  async startServer(): Promise<number> {
     if (!existsSync(PROTO_PATH)) {
-      return Promise.reject(new Error(`zemu.proto not found at ${PROTO_PATH}`))
+      throw new Error(`zemu.proto not found at ${PROTO_PATH}`)
     }
 
     const packageDefinition = loadSync(PROTO_PATH, {
@@ -44,14 +44,14 @@ export default class GRPCRouter {
       },
     })
 
-    return new Promise<void>((resolvePromise, reject) => {
+    return await new Promise<number>((resolvePromise, reject) => {
       this.server.bindAsync(this.serverAddress, ServerCredentials.createInsecure(), (err, port) => {
         if (err != null) {
           reject(err)
           return
         }
         process.stdout.write(`gRPC listening on ${port}\n`)
-        resolvePromise()
+        resolvePromise(port)
       })
     })
   }

src/grpc/index.ts

@@ -18,6 +18,7 @@ import { PolkadotGenericApp } from '@zondax/ledger-substrate'
 import { describe, expect, test } from 'vitest'
 import Zemu, { zondaxMainmenuNavigation } from '../src'
 import { defaultOptions, models, nanoModels, PATH, POLYMESH_SS58_PREFIX, SNAPSHOTS_DIR } from './common'
+import { exchangeViaGrpc } from './grpcClient'
 
 // DER prefix for a raw Ed25519 public key (SubjectPublicKeyInfo)
 const ED25519_SPKI_PREFIX = Buffer.from('302a300506032b6570032100', 'hex')
@@ -159,13 +160,22 @@ describe.each(nanoModels)('$name buttons', (m) => {
   })
 })
 
-test('gRPC server start-stop', async () => {
+test('gRPC server forwards APDUs to the device', async () => {
   const m = nanoModels[0]
   const sim = new Zemu(m.path)
   try {
     await sim.start({ ...defaultOptions, model: m.name })
-    await sim.startGRPCServer('127.0.0.1', 0)
+    const port = await sim.startGRPCServer('127.0.0.1', 0)
+    expect(port).toBeGreaterThan(0)
+
+    // GET_APP_INFO (handled by the OS): format id 1, then the app name
+    const reply = await exchangeViaGrpc(port, Buffer.from([0xb0, 0x01, 0x00, 0x00, 0x00]))
+    expect(reply.readUInt16BE(reply.length - 2)).toBe(0x9000)
+    expect(reply[0]).toBe(1)
+    expect(reply.subarray(2, 2 + reply[1]).toString('ascii')).toBe('Polymesh')
+
     sim.stopGRPCServer()
+    await expect(exchangeViaGrpc(port, Buffer.alloc(0))).rejects.toThrow()
   } finally {
     await sim.close()
   }

tests/basic.test.ts

@@ -37,9 +37,10 @@ describe('APDU error handling', () => {
       await sim.start(options)
       const transport = sim.getTransport()
 
+      // Generous bound: the point is "not a wait-helper timeout", not raw latency
       const startTime = Date.now()
       await expect(transport.send(INVALID_CLA, 0x00, 0x00, 0x00)).rejects.toMatchObject({ statusCode: APDU_STATUS_CODES.CLA_NOT_SUPPORTED })
-      expect(Date.now() - startTime).toBeLessThan(2000)
+      expect(Date.now() - startTime).toBeLessThan(5000)
 
       const recorded = sim.getLastTransportError() as any
       expect(recorded).not.toBeNull()
@@ -76,11 +77,12 @@ describe('APDU error handling', () => {
 
       await expect(transport.send(INVALID_CLA, 0x00, 0x00, 0x00)).rejects.toThrow()
 
+      // Must fail well before the 20s wait timeout
       const startTime = Date.now()
-      await expect(sim.waitUntilScreenIs(differentScreen(sim), 5000)).rejects.toBeInstanceOf(TransportError)
-      expect(Date.now() - startTime).toBeLessThan(1500)
+      await expect(sim.waitUntilScreenIs(differentScreen(sim), 20000)).rejects.toBeInstanceOf(TransportError)
+      expect(Date.now() - startTime).toBeLessThan(5000)
 
-      await expect(sim.waitForText('never shown', 5000)).rejects.toBeInstanceOf(TransportError)
+      await expect(sim.waitForText('never shown', 20000)).rejects.toBeInstanceOf(TransportError)
       await expect(sim.getEvents()).rejects.toMatchObject({ statusCode: APDU_STATUS_CODES.CLA_NOT_SUPPORTED })
     } finally {
       await sim.close()

tests/error-handling.test.ts

@@ -2,16 +2,21 @@ import Zemu from '../src'
 
 // Registered in vitest.config.ts as globalSetup. Runs once in the main vitest process.
 
-async function killContainers(reason: string): Promise<void> {
+// Only containers created by this run are swept on teardown, so a concurrent
+// zemu session on the same Docker daemon is left alone. Ctrl-C still sweeps everything.
+let runStartedAt = 0
+
+async function killContainers(reason: string, createdAfter?: number): Promise<void> {
   console.log(`[zemu] ${reason}: stopping dangling containers`)
   try {
-    await Zemu.stopAllEmuContainers()
+    await Zemu.stopAllEmuContainers(createdAfter)
   } catch (error) {
     console.error('[zemu] failed to stop containers:', error)
   }
 }
 
 export function setup(): void {
+  runStartedAt = Math.floor(Date.now() / 1000) - 1
   for (const signal of ['SIGINT', 'SIGTERM'] as const) {
     process.once(signal, () => {
       void killContainers(signal).finally(() => process.exit(130))
@@ -20,5 +25,5 @@ export function setup(): void {
 }
 
 export async function teardown(): Promise<void> {
-  await killContainers('teardown')
+  await killContainers('teardown', runStartedAt)
 }