fix(bb-prover): replace a dead pooled bb verifier, by letting the instance heal itself

yarn-project/bb-prover/src/bb/bb_js_backend.test.ts

@@ -1,5 +1,7 @@
+import { promiseWithResolvers } from '@aztec-labs/foundation/promise';
 import { ProvingError } from '@aztec-labs/stdlib/errors';
 
+import { FakeBBJsFactory } from '../test/fake_bb_js.js';
 import { BBJsInstance } from './bb_js_backend.js';
 
 describe('BBJsInstance', () => {
@@ -9,3 +11,56 @@ describe('BBJsInstance', () => {
     expect(err.retry).toBe(true);
   });
 });
+
+describe('BBJsFactory pool', () => {
+  const tick = () => new Promise(resolve => setImmediate(resolve));
+
+  it('reuses its instances, starting no more than the pool holds', async () => {
+    const factory = new FakeBBJsFactory(1);
+    const borrow = async () => {
+      await using _instance = await factory.getInstance();
+      await tick();
+    };
+    await Promise.all([borrow(), borrow(), borrow()]);
+    expect(factory.created).toHaveLength(1);
+
+    await factory.destroy();
+    expect(factory.created[0].destroyCount).toBe(1);
+  });
+
+  it('gives a slot whose bb failed to start to the next borrower, which starts it again', async () => {
+    const factory = new FakeBBJsFactory(1);
+    factory.planNextInstance(new Error('spawn failed'));
+    await expect(factory.getInstance()).rejects.toThrow('spawn failed');
+
+    await using _instance = await factory.getInstance();
+    expect(factory.created).toHaveLength(1);
+  });
+
+  it('releases a borrower waiting for a slot at once on destroy, and one starting bb when its start ends', async () => {
+    const factory = new FakeBBJsFactory(1);
+    const start = promiseWithResolvers<void>();
+    factory.planNextInstance([], start.promise);
+    const starting = factory.getInstance();
+    const waiting = factory.getInstance();
+    await tick();
+
+    const destroying = factory.destroy();
+    await expect(waiting).rejects.toThrow(/destroyed/);
+    start.resolve();
+    await expect(starting).rejects.toThrow(/destroyed/);
+    await destroying;
+    expect(factory.created[0].destroyCount).toBe(1);
+  });
+
+  it('destroys an instance borrowed across destroy when it is released, rather than pooling it', async () => {
+    const factory = new FakeBBJsFactory(1);
+    const instance = await factory.getInstance();
+    await factory.destroy();
+    expect(factory.created[0].destroyCount).toBe(0);
+
+    await instance[Symbol.asyncDispose]();
+    expect(factory.created[0].destroyCount).toBe(1);
+    await expect(factory.getInstance()).rejects.toThrow(/destroyed/);
+  });
+});

yarn-project/bb-prover/src/bb/bb_js_backend.ts

@@ -76,12 +76,19 @@ export interface BBJsApi {
 export class BBJsInstance implements BBJsApi {
   private constructor(private api: Barretenberg) {}
 
-  /** Creates a new Barretenberg instance connected to a fresh bb process. */
-  static async create(bbPath: string, logger?: LogFn, threads?: number): Promise<BBJsInstance> {
+  /**
+   * Creates a new Barretenberg instance connected to a fresh bb process.
+   *
+   * `respawn` lets the instance replace its bb process when it dies, so a long-lived instance stays
+   * usable instead of failing every later call. Only safe where the instance holds no state between
+   * calls, since a replacement has no Chonk accumulation and no batch-verifier session.
+   */
+  static async create(bbPath: string, logger?: LogFn, threads?: number, respawn?: boolean): Promise<BBJsInstance> {
     const options: BackendOptions = {
       bbPath,
       backend: BackendType.NativeUnixSocket,
       logger,
+      respawn,
     };
     if (threads !== undefined) {
       options.threads = threads;
@@ -251,17 +258,31 @@ export interface BBJsFactoryOptions {
    * If omitted, every `getInstance()` call spawns a fresh bb that is destroyed on dispose.
    */
   poolSize?: number;
+  /**
+   * Let each instance replace its bb process when it dies. Only for callers whose calls stand alone:
+   * a replacement process remembers nothing, so state held across calls (a Chonk accumulation, a
+   * batch-verifier session) would be silently lost.
+   */
+  respawn?: boolean;
   logger?: Logger;
   threads?: number;
   debugDir?: string;
 }
 
+/** A place in a {@link BBJsFactory} pool: empty until a borrower first starts a bb in it. */
+type PoolSlot = { instance?: BBJsApi };
+
 /**
  * Manages bb.js instance lifecycle. By default every `getInstance()` call spawns a fresh
  * bb process that is destroyed when the borrow is disposed. Pass `poolSize` to keep a fixed
  * set of long-lived bb processes that are reused across calls — useful when the per-call
  * bb startup cost dominates the workload (e.g. high-rate IVC verification).
  *
+ * The pool is a queue of `poolSize` slots, each starting its bb on the first borrow that needs it.
+ * A borrower waits in exactly two places: for a slot, which destroy() releases by cancelling the
+ * queue, and for its slot's bb to start, which bb.js bounds with its own startup deadline. A bb that
+ * fails to start costs only that slot, which goes back empty for the next borrower to try again.
+ *
  * Idiomatic usage:
  * ```
  * await using inst = await factory.getInstance();
@@ -270,82 +291,86 @@ export interface BBJsFactoryOptions {
  * ```
  */
 export class BBJsFactory {
-  private readonly poolSize?: number;
   private readonly logger?: Logger;
   private readonly threads?: number;
   private readonly debugDir?: string;
+  private readonly respawn: boolean;
 
-  /** Available pooled instances when poolSize is set; otherwise undefined. */
-  private pool?: FifoMemoryQueue<BBJsApi>;
-  /** Lazily-resolved on first `getInstance()` call to prevent racing pool initialization. */
-  private initPromise?: Promise<void>;
+  /** Slots not currently borrowed, when poolSize is set; otherwise undefined. */
+  private readonly slots?: FifoMemoryQueue<PoolSlot>;
   private destroyed = false;
 
   constructor(
     private bbPath: string,
     options: BBJsFactoryOptions = {},
   ) {
-    this.poolSize = options.poolSize;
+    this.respawn = options.respawn ?? false;
     this.logger = options.logger;
     this.threads = options.threads;
     this.debugDir = options.debugDir;
-    if (this.poolSize !== undefined && this.poolSize < 1) {
-      throw new Error(`BBJsFactory poolSize must be >= 1, got ${this.poolSize}`);
+    if (options.poolSize !== undefined) {
+      if (options.poolSize < 1) {
+        throw new Error(`BBJsFactory poolSize must be >= 1, got ${options.poolSize}`);
+      }
+      this.slots = new FifoMemoryQueue<PoolSlot>();
+      for (let i = 0; i < options.poolSize; i++) {
+        this.slots.put({});
+      }
     }
   }
 
   /**
    * Acquire a bb instance. The returned object implements `BBJsApi` and `AsyncDisposable`.
-   * With no pool: spawns a fresh bb that is destroyed on dispose. With a pool: borrows from
-   * the pool and returns to it on dispose.
+   * With no pool: spawns a fresh bb that is destroyed on dispose. With a pool: borrows a slot,
+   * starting its bb if it has none, and returns the slot to the pool on dispose.
    */
   async getInstance(): Promise<BBJsApi & AsyncDisposable> {
     if (this.destroyed) {
       throw new Error('BBJsFactory has been destroyed');
     }
-    if (this.poolSize === undefined) {
+    if (!this.slots) {
       // No pool: fresh-per-call, dispose destroys.
       const instance = await this.createInstance();
       return this.makeOwned(instance);
     }
-    if (!this.initPromise) {
-      this.initPromise = this.initPool();
+    const slot = await this.slots.get();
+    if (!slot) {
+      throw new Error('BBJsFactory was destroyed while waiting for an instance');
     }
-    await this.initPromise;
-    const pool = this.pool;
-    if (!pool) {
-      throw new Error('BBJsFactory has been destroyed');
+    let instance: BBJsApi;
+    try {
+      instance = slot.instance ??= await this.createInstance();
+    } catch (err) {
+      await this.release(slot);
+      throw err;
     }
-    const instance = await pool.get();
-    if (!instance) {
-      throw new Error('BBJsFactory was destroyed while waiting for an instance');
+    if (this.destroyed) {
+      await this.release(slot);
+      throw new Error('BBJsFactory has been destroyed');
     }
-    return this.makeBorrowed(instance);
+    return this.makeDisposable(instance, () => this.release(slot));
   }
 
   /**
    * Tear down all pooled instances. Idempotent. No-op when no pool is configured (fresh-per-call
-   * instances are destroyed by their own dispose callbacks). Instances currently held by an
-   * in-flight pooled borrow are destroyed by their dispose callback when released.
+   * instances are destroyed by their own dispose callbacks). Instances in borrowed slots are
+   * destroyed when their borrow is released.
    */
   async destroy(): Promise<void> {
     if (this.destroyed) {
       return;
     }
     this.destroyed = true;
-    const pool = this.pool;
-    this.pool = undefined;
-    if (!pool) {
+    if (!this.slots) {
       return;
     }
     const idle: BBJsApi[] = [];
-    while (pool.length() > 0) {
-      const item = pool.getImmediate();
-      if (item) {
-        idle.push(item);
+    for (let slot = this.slots.getImmediate(); slot; slot = this.slots.getImmediate()) {
+      if (slot.instance) {
+        idle.push(slot.instance);
       }
     }
-    pool.cancel();
+    this.slots.cancel();
     // Aggregate teardown failures so a single bb child that fails to shut down doesn't mask others.
     const results = await Promise.allSettled(idle.map(item => item.destroy()));
     const errors = results.filter((r): r is PromiseRejectedResult => r.status === 'rejected').map(r => r.reason);
@@ -354,37 +379,18 @@ export class BBJsFactory {
     }
   }
 
-  private async initPool(): Promise<void> {
-    // Use allSettled so that if any createInstance() rejects we can destroy the rest instead of
-    // leaking bb child processes whose creation succeeded.
-    const results = await Promise.allSettled(Array.from({ length: this.poolSize! }, () => this.createInstance()));
-    const items: BBJsApi[] = [];
-    const errors: unknown[] = [];
-    for (const result of results) {
-      if (result.status === 'fulfilled') {
-        items.push(result.value);
-      } else {
-        errors.push(result.reason);
-      }
-    }
-    if (errors.length > 0 || this.destroyed) {
-      // Either creation failed or destroy() raced ahead — clean up everything we successfully spawned.
-      await Promise.all(items.map(item => item.destroy()));
-      if (errors.length > 0) {
-        throw errors[0];
-      }
-      return;
-    }
-    const pool = new FifoMemoryQueue<BBJsApi>();
-    for (const item of items) {
-      pool.put(item);
+  /** Return a borrowed slot to the pool, or destroy its bb if the factory was destroyed meanwhile. */
+  private async release(slot: PoolSlot): Promise<void> {
+    if (!this.destroyed) {
+      this.slots!.put(slot);
+    } else {
+      await slot.instance?.destroy();
     }
-    this.pool = pool;
   }
 
-  private async createInstance(): Promise<BBJsApi> {
+  protected async createInstance(): Promise<BBJsApi> {
     const logFn = this.logger ? (msg: string) => this.logger!.verbose(`bb.js - ${msg}`) : undefined;
-    const raw = await BBJsInstance.create(this.bbPath, logFn, this.threads);
+    const raw = await BBJsInstance.create(this.bbPath, logFn, this.threads, this.respawn);
     return this.maybeWrapDebug(raw);
   }
 
@@ -406,21 +412,6 @@ export class BBJsFactory {
     return this.makeDisposable(instance, () => instance.destroy());
   }
 
-  /**
-   * Wrap a pooled instance with an `AsyncDisposable` that returns it to the pool (or destroys it
-   * if the factory was destroyed in the meantime). Destroy errors are propagated.
-   */
-  private makeBorrowed(instance: BBJsApi): BBJsApi & AsyncDisposable {
-    return this.makeDisposable(instance, async () => {
-      const pool = this.pool;
-      if (pool && !this.destroyed) {
-        pool.put(instance);
-      } else {
-        await instance.destroy();
-      }
-    });
-  }
-
   private makeDisposable(instance: BBJsApi, onDispose: () => void | Promise<void>): BBJsApi & AsyncDisposable {
     let disposed = false;
     const dispose = async (): Promise<void> => {

yarn-project/bb-prover/src/prover/server/bb_prover.ts

@@ -9,6 +9,7 @@ import {
   ULTRA_KECCAK_PROOF_LENGTH,
 } from '@aztec-labs/constants';
 import { Fr } from '@aztec-labs/foundation/curves/bn254';
+import { isRetryableError } from '@aztec-labs/foundation/error';
 import { runInDirectory } from '@aztec-labs/foundation/fs';
 import { createLogger } from '@aztec-labs/foundation/log';
 import {
@@ -449,7 +450,7 @@ export class BBNativeRollupProver implements ServerCircuitProver {
       );
     } catch (error) {
       // Preserve retryability of the underlying failure (e.g. a transient bb startup error).
-      const retry = error instanceof ProvingError && error.retry;
+      const retry = isRetryableError(error);
       throw new ProvingError(`Failed to generate proof for ${circuitType}: ${error}`, error, retry);
     }
 
@@ -590,7 +591,7 @@ export class BBNativeRollupProver implements ServerCircuitProver {
       ));
     } catch (error) {
       // Preserve retryability of the underlying failure (e.g. a transient bb startup error).
-      const retry = error instanceof ProvingError && error.retry;
+      const retry = isRetryableError(error);
       throw new ProvingError(`Failed to verify proof for ${circuitType}: ${error}`, error, retry);
     }