Skip to content

Commit 543b69c

Browse files
authored
chore: Remove --forceExit from yarn-project tests. (#20890)
Using `--forceExit` is a hack that potentially hides leaked handles that keep node alive when you want it to exit. This removes force exit, but requires another hack to workaround the fact we use viem, which leaks timer handles and pipes (according to Claude. Don't want to prematurely appoint blame if we're just using it wrong, but Claude seemed to go quite deep). * We replace viem/anvil with our own direct launcher drop-in replacement. WARNING This was a total vibe code. It works though and got rid of viems leaks. * We add a jest setup that hooks setTimeout and setInterval to unref their handles. This is bit of a brute force hack, but viem leaks timers, and there's no obvious reason to let timers keep tests running. It also adds a diagnostic block that can give some info in the event of leaks (like jests --detectLeakedHandles or whatever). * Lazier init of the crates kzg lib as that looked like it might be a culprit. Basically can be interpreted as `--forceExitIfLeakedTimers`... This means that other handle leaks will prevent tests from exiting, so they can be correctly resolved. Also: * Adds a test for signal propagation within ci3 (regressed)
2 parents da4b3b4 + da15126 commit 543b69c

57 files changed

Lines changed: 493 additions & 217 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

barretenberg/ts/src/barretenberg/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ export class Barretenberg extends AsyncApi {
119119
*/
120120
static async initSingleton(options: BackendOptions = {}) {
121121
if (!barretenbergSingletonPromise) {
122-
barretenbergSingletonPromise = Barretenberg.new(options);
122+
barretenbergSingletonPromise = Barretenberg.new({ ...options, unref: true });
123123
}
124124
try {
125125
barretenbergSingleton = await barretenbergSingletonPromise;

barretenberg/ts/src/barretenberg_wasm/barretenberg_wasm_main/index.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export class BarretenbergWasmMain extends BarretenbergWasmBase {
3737
logger?: (msg: string) => void,
3838
initial = 35,
3939
maximum = this.getDefaultMaximumMemoryPages(),
40+
unref = false,
4041
) {
4142
// Track whether a custom logger was provided so workers know whether to postMessage logs
4243
this.useCustomLogger = logger !== undefined;
@@ -81,6 +82,12 @@ export class BarretenbergWasmMain extends BarretenbergWasmBase {
8182

8283
this.remoteWasms = await Promise.all(this.workers.map(getRemoteBarretenbergWasm<BarretenbergWasmThreadWorker>));
8384
await Promise.all(this.remoteWasms.map(w => w.initThread(module, this.memory, this.useCustomLogger)));
85+
86+
if (unref) {
87+
for (const worker of this.workers) {
88+
worker.unref();
89+
}
90+
}
8491
}
8592
}
8693

barretenberg/ts/src/bb_backends/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,4 +52,12 @@ export type BackendOptions = {
5252
* BarretenbergSync supports: Wasm, NativeSharedMem only
5353
*/
5454
backend?: BackendType;
55+
56+
/**
57+
* @description Mark backend handles (worker threads, sockets, pipes) as unref'd so they
58+
* don't prevent the Node.js process from exiting. Used for the singleton instance where
59+
* callers don't manage the lifecycle. Non-singleton instances should leave this false
60+
* and call destroy() to clean up.
61+
*/
62+
unref?: boolean;
5563
};

barretenberg/ts/src/bb_backends/node/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export async function createAsyncBackend(
2626
throw new Error('Native backend requires bb binary.');
2727
}
2828
logger(`Using native Unix socket backend: ${bbPath}`);
29-
const socket = new BarretenbergNativeSocketAsyncBackend(bbPath, options.threads, options.logger);
29+
const socket = new BarretenbergNativeSocketAsyncBackend(bbPath, options.threads, options.logger, options.unref);
3030
return new Barretenberg(socket, options);
3131
}
3232

@@ -59,6 +59,7 @@ export async function createAsyncBackend(
5959
logger: options.logger,
6060
memory: options.memory,
6161
useWorker,
62+
unref: options.unref,
6263
});
6364
return new Barretenberg(wasm, options);
6465
}

barretenberg/ts/src/bb_backends/node/native_socket.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ export class BarretenbergNativeSocketAsyncBackend implements IMsgpackBackendAsyn
4343
private responseBuffer: Buffer | null = null;
4444
private responseBytesRead: number = 0;
4545

46-
constructor(bbBinaryPath: string, threads?: number, logger?: (msg: string) => void) {
46+
constructor(bbBinaryPath: string, threads?: number, logger?: (msg: string) => void, unref?: boolean) {
4747
// Create a unique socket path in temp directory
4848
this.socketPath = path.join(os.tmpdir(), `bb-${process.pid}-${threadId}-${instanceCounter++}.sock`);
4949

@@ -80,6 +80,10 @@ export class BarretenbergNativeSocketAsyncBackend implements IMsgpackBackendAsyn
8080
logger("Logger attached to bb process. DON'T FORGET TO DESTROY THE BACKEND to allow Node.js to exit.");
8181
readline.createInterface({ input: this.process.stdout! }).on('line', logger);
8282
readline.createInterface({ input: this.process.stderr! }).on('line', logger);
83+
if (unref) {
84+
(this.process.stdout as any)?.unref?.();
85+
(this.process.stderr as any)?.unref?.();
86+
}
8387
}
8488

8589
this.process.on('error', err => {

barretenberg/ts/src/bb_backends/wasm.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ export class BarretenbergWasmAsyncBackend implements IMsgpackBackendAsync {
5454
* @param options.logger Optional logging function
5555
* @param options.memory Optional initial and maximum memory configuration
5656
* @param options.useWorker Run on worker thread (default: true for browser safety)
57+
* @param options.unref Unref worker handles so they don't prevent process exit
5758
*/
5859
static async new(
5960
options: {
@@ -62,6 +63,7 @@ export class BarretenbergWasmAsyncBackend implements IMsgpackBackendAsync {
6263
logger?: (msg: string) => void;
6364
memory?: { initial?: number; maximum?: number };
6465
useWorker?: boolean;
66+
unref?: boolean;
6567
} = {},
6668
): Promise<BarretenbergWasmAsyncBackend> {
6769
// Default to worker mode for browser safety
@@ -79,12 +81,15 @@ export class BarretenbergWasmAsyncBackend implements IMsgpackBackendAsync {
7981
options.memory?.initial,
8082
options.memory?.maximum,
8183
);
84+
if (options.unref) {
85+
worker.unref();
86+
}
8287
return new BarretenbergWasmAsyncBackend(wasm, worker);
8388
} else {
8489
// Direct mode: runs on calling thread (faster but blocks thread)
8590
const wasm = new BarretenbergWasmMain();
8691
const { module, threads } = await fetchModuleAndThreads(options.threads, options.wasmPath, options.logger);
87-
await wasm.init(module, threads, options.logger, options.memory?.initial, options.memory?.maximum);
92+
await wasm.init(module, threads, options.logger, options.memory?.initial, options.memory?.maximum, options.unref);
8893
return new BarretenbergWasmAsyncBackend(wasm);
8994
}
9095
}

ci3/bootstrap.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ hash=$(cache_content_hash ^ci3)
55

66
function test_cmds {
77
echo "$hash $ci3/tests/redact_test"
8+
echo "$hash $ci3/tests/signal_test"
89
echo "$hash $ci3/semver test"
910
}
1011

ci3/run_test_cmd

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,12 @@ function cleanup {
102102
}
103103
trap cleanup EXIT
104104

105-
# Signal handler to forward SIGTERM/SIGINT to test process.
105+
# Signal handler to forward SIGTERM/SIGINT to test process group.
106+
# We use set -m so exec_test runs in its own process group (leader PID = test_pid).
107+
# We must kill the process GROUP (-$pid) not just exec_test, otherwise children
108+
# like docker_isolate are orphaned and Docker containers keep running.
106109
function sig_handler {
107-
kill -TERM ${test_pid:-} &>/dev/null
110+
[ -n "${test_pid:-}" ] && kill -TERM -$test_pid &>/dev/null
108111
exit
109112
}
110113
trap sig_handler SIGTERM SIGINT

ci3/tests/signal_test

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
#!/usr/bin/env bash
2+
# Test that SIGTERM propagates through the full CI pipeline and kills Docker containers.
3+
#
4+
# When tests run with ISOLATE=1, they execute inside a Docker container via docker_isolate.
5+
# If the pipeline receives SIGTERM (e.g., Ctrl-C during `parallelize`), the container must
6+
# be killed.
7+
#
8+
# The full signal flow on Ctrl-C:
9+
# Terminal sends SIGINT to foreground process group
10+
# → parallel, run_test_cmd receive signal (same process group)
11+
# → run_test_cmd sig_handler fires
12+
# → must send SIGTERM to exec_test's process GROUP (not just exec_test)
13+
# → docker_isolate receives SIGTERM → cleanup trap → docker rm -f
14+
#
15+
# The critical detail: run_test_cmd uses set -m, so exec_test runs in its
16+
# OWN process group. The sig_handler must use kill -TERM -$pid (negative = group),
17+
# otherwise docker_isolate is orphaned and the container keeps running.
18+
set -euo pipefail
19+
20+
source "$(git rev-parse --show-toplevel)/ci3/source"
21+
22+
RED=$'\033[0;31m'
23+
GREEN=$'\033[0;32m'
24+
NC=$'\033[0m'
25+
26+
TESTS_PASSED=0
27+
TESTS_FAILED=0
28+
29+
# Unique prefix to avoid container name collisions.
30+
PREFIX="ci3sigtest$$"
31+
32+
function pass { echo "${GREEN}${NC} $1"; ((TESTS_PASSED++)) || true; }
33+
function fail {
34+
echo "${RED}${NC} $1: $2"
35+
((TESTS_FAILED++)) || true
36+
}
37+
38+
# Wait for a container to appear (up to N seconds).
39+
function wait_for_container {
40+
local name=$1 max=${2:-15}
41+
for ((i=0; i<max; i++)); do
42+
if docker ps --filter "name=^${name}$" --format '{{.Names}}' | grep -q "^${name}$"; then
43+
return 0
44+
fi
45+
sleep 1
46+
done
47+
return 1
48+
}
49+
50+
# Check that a container has stopped (within N seconds).
51+
function wait_for_no_container {
52+
local name=$1 max=${2:-10}
53+
for ((i=0; i<max; i++)); do
54+
if ! docker ps --filter "name=^${name}$" --format '{{.Names}}' | grep -q "^${name}$"; then
55+
return 0
56+
fi
57+
sleep 1
58+
done
59+
return 1
60+
}
61+
62+
# Cleanup any leftover containers on exit.
63+
function cleanup_all {
64+
docker ps -a --filter "name=$PREFIX" --format '{{.Names}}' | while read -r name; do
65+
docker rm -f "$name" &>/dev/null || true
66+
done
67+
}
68+
trap cleanup_all EXIT
69+
70+
# Verify docker is available and the build image exists.
71+
if ! docker info &>/dev/null; then
72+
echo "SKIP: Docker not available"
73+
exit 0
74+
fi
75+
76+
if ! docker image inspect aztecprotocol/build:3.0 &>/dev/null; then
77+
echo "SKIP: aztecprotocol/build:3.0 image not available"
78+
exit 0
79+
fi
80+
81+
echo "=== Signal Propagation Tests ==="
82+
83+
# ─── Test: full pipeline (parallelize) SIGTERM kills container ──────
84+
#
85+
# Mirrors the real Ctrl-C scenario:
86+
# echo "hash:ISOLATE=1 sleep 60" | parallelize
87+
# User presses Ctrl-C → SIGINT to foreground process group.
88+
#
89+
# We use set -m so the backgrounded pipeline gets its own process group,
90+
# then kill the group (simulating Ctrl-C to the foreground group).
91+
# This reaches parallel and run_test_cmd, but NOT exec_test/docker_isolate
92+
# (they're in a separate group due to run_test_cmd's set -m).
93+
# The test passes only if run_test_cmd's sig_handler propagates to docker_isolate.
94+
echo "--- Test: echo '...' | parallelize receives SIGTERM ---"
95+
name="${PREFIX}t1"
96+
97+
# Job control: backgrounded pipeline gets its own process group.
98+
set -m
99+
echo "fakehash:ISOLATE=1:CPUS=1:NAME=${name}:TIMEOUT=120s sleep 60" | parallelize 1 &>/dev/null &
100+
pid=$!
101+
set +m
102+
103+
if wait_for_container "$name"; then
104+
# Kill the pipeline's process group (simulates Ctrl-C).
105+
kill -TERM -- -$pid 2>/dev/null || true
106+
wait $pid 2>/dev/null || true
107+
if wait_for_no_container "$name"; then
108+
pass "parallelize SIGTERM propagation"
109+
else
110+
fail "parallelize SIGTERM propagation" "Container still running after SIGTERM"
111+
fi
112+
else
113+
fail "parallelize SIGTERM propagation" "Container never started"
114+
fi
115+
116+
echo
117+
if [[ $TESTS_FAILED -eq 0 ]]; then
118+
echo "${GREEN}All $TESTS_PASSED tests passed.${NC}"
119+
exit 0
120+
else
121+
echo "${RED}$TESTS_FAILED of $((TESTS_PASSED + TESTS_FAILED)) tests failed.${NC}"
122+
exit 1
123+
fi

yarn-project/blob-lib/src/blob.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { updateInlineTestData } from '@aztec/foundation/testing/files';
66

77
import { Blob } from './blob.js';
88
import { commitmentToFields } from './hash.js';
9-
import { BYTES_PER_BLOB, getKzg } from './kzg_context.js';
9+
import { getBytesPerBlob, getKzg } from './kzg_context.js';
1010
import { makeRandomBlob } from './testing.js';
1111

1212
describe('blob', () => {
@@ -19,9 +19,9 @@ describe('blob', () => {
1919
const kzgProofs: Uint8Array[] = [];
2020

2121
for (let i = 0; i < BATCH_SIZE; i++) {
22-
blobs.push(Buffer.alloc(BYTES_PER_BLOB));
22+
blobs.push(Buffer.alloc(getBytesPerBlob()));
2323
(blobs[i] as Buffer).write('potato', 0, 'utf8');
24-
(blobs[i] as Buffer).write('potato', BYTES_PER_BLOB - 50, 'utf8');
24+
(blobs[i] as Buffer).write('potato', getBytesPerBlob() - 50, 'utf8');
2525
commitments.push(kzg.blobToKzgCommitment(blobs[i]));
2626
kzgProofs.push(kzg.computeBlobKzgProof(blobs[i], commitments[i]));
2727
}
@@ -42,7 +42,7 @@ describe('blob', () => {
4242
// This is the 2nd root of unity, after 1, because we actually get the bit_reversal_permutation of the root of unity. And although `7` is the primitive root of unity, the roots of unity are derived as 7 ^ ((BLS_MODULUS - 1) / FIELDS_PER_BLOB) mod BLS_MODULUS.
4343
(zBytes as Buffer).write('73EDA753299D7D483339D80809A1D80553BDA402FFFE5BFEFFFFFFFF00000000', 0, 'hex'); // equiv to 52435875175126190479447740508185965837690552500527637822603658699938581184512 which is actually -1 in the scalar field!
4444

45-
const blob = Buffer.alloc(BYTES_PER_BLOB);
45+
const blob = Buffer.alloc(getBytesPerBlob());
4646
(blob as Buffer).write('09', 31, 'hex');
4747
(blob as Buffer).write('07', 31 + 32, 'hex');
4848

0 commit comments

Comments
 (0)