Skip to content

Commit 18f5dfe

Browse files
committed
Merge remote-tracking branch 'origin/main' into t3code/fix-arbitrager-old-logs
# Conflicts: # bots/shared/tests/shared-primitives.test.ts
2 parents 46d3eca + 23c9ef3 commit 18f5dfe

5 files changed

Lines changed: 186 additions & 41 deletions

File tree

bots/liquidator/Dockerfile

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,15 @@ RUN cd shared \
3333
COPY bots/liquidator/README.md ./bots/liquidator/README.md
3434
COPY bots/liquidator/config/ ./bots/liquidator/config/
3535
COPY bots/liquidator/scripts/check-market-sources.mts ./bots/liquidator/scripts/check-market-sources.mts
36+
COPY bots/liquidator/scripts/check-process-lock-runtime.mts ./bots/liquidator/scripts/check-process-lock-runtime.mts
3637
COPY bots/liquidator/scripts/docker-entrypoint.sh ./bots/liquidator/scripts/docker-entrypoint.sh
3738
COPY bots/liquidator/src/ ./bots/liquidator/src/
3839
RUN chmod 0755 bots/liquidator/scripts/docker-entrypoint.sh \
3940
&& install -d -m 0700 -o bun -g bun bots/liquidator/.state
4041

4142
USER bun
4243
WORKDIR /app/bots/liquidator
44+
RUN bun ./scripts/check-process-lock-runtime.mts
4345

4446
EXPOSE 4183
4547
VOLUME ["/app/bots/liquidator/.state"]
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { acquireExclusiveProcessLock } from '@zoltar/bot-shared/execution/process-lock'
2+
import { mkdtemp, rm, stat } from 'node:fs/promises'
3+
import { tmpdir } from 'node:os'
4+
import { join } from 'node:path'
5+
6+
const directory = await mkdtemp(join(tmpdir(), 'zoltar-process-lock-runtime-'))
7+
const lockPath = join(directory, 'operator.lock')
8+
9+
try {
10+
const first = await acquireExclusiveProcessLock(lockPath, 'Runtime check lock', {})
11+
try {
12+
await acquireExclusiveProcessLock(lockPath, 'Runtime check lock', {})
13+
throw new Error('A live process lock allowed a competing owner')
14+
} catch (error) {
15+
if (!(error instanceof Error) || !error.message.includes('already locked')) throw error
16+
}
17+
const inode = (await stat(lockPath)).ino
18+
await first.release()
19+
if ((await stat(lockPath)).ino !== inode) throw new Error('Process lock release replaced the stable lock inode')
20+
21+
const child = Bun.spawn([process.execPath, '--eval', `import { acquireExclusiveProcessLock } from '@zoltar/bot-shared/execution/process-lock'; await acquireExclusiveProcessLock(${JSON.stringify(lockPath)}, 'Runtime check lock', {}); console.log('ready'); await Bun.sleep(60_000)`], {
22+
cwd: import.meta.dir,
23+
stderr: 'pipe',
24+
stdout: 'pipe',
25+
})
26+
try {
27+
const reader = child.stdout.getReader()
28+
const next = await reader.read()
29+
if (next.done || !new TextDecoder().decode(next.value).includes('ready')) throw new Error(`Process-lock runtime child stopped before acquisition: ${await new Response(child.stderr).text()}`)
30+
reader.releaseLock()
31+
child.kill('SIGKILL')
32+
if ((await child.exited) === 0) throw new Error('Process-lock runtime child was not killed')
33+
} finally {
34+
if (child.exitCode === null) child.kill('SIGKILL')
35+
await child.exited
36+
}
37+
38+
const replacement = await acquireExclusiveProcessLock(lockPath, 'Runtime check lock', {})
39+
await replacement.release()
40+
} finally {
41+
await rm(directory, { force: true, recursive: true })
42+
}

bots/liquidator/tests/docker-packaging.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ describe('Docker packaging', () => {
4242
expect(source).toContain('&& bun run shared:build')
4343
expect(source).toContain('COPY --from=shared-builder /source/shared/ ./shared/')
4444
expect(source).toContain('cd shared \\\n\t&& bun install --frozen-lockfile --production \\\n\t&& cd ../bots/shared \\\n\t&& bun install --frozen-lockfile --production')
45+
expect(source).toContain('COPY bots/liquidator/scripts/check-process-lock-runtime.mts ./bots/liquidator/scripts/check-process-lock-runtime.mts')
46+
expect(source).toContain('RUN bun ./scripts/check-process-lock-runtime.mts')
4547
})
4648

4749
test('starts without host UID, GID, or .env configuration', async () => {

bots/shared/src/execution/process-lock.ts

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,47 @@
1-
import { lstat, mkdir, open, readFile, rm } from 'node:fs/promises'
1+
import { constants } from 'node:fs'
2+
import { lstat, mkdir, open, readFile } from 'node:fs/promises'
23
import { tmpdir } from 'node:os'
34
import { dirname, join, resolve } from 'node:path'
5+
import { dlopen } from 'bun:ffi'
46
import { getAddress, type Address } from '../ethereum.ts'
57

68
type ProcessLockFileHandle = {
79
chmod: (mode: number) => Promise<unknown>
810
close: () => Promise<unknown>
11+
fd: number
912
sync: () => Promise<unknown>
13+
truncate: (length?: number) => Promise<unknown>
1014
writeFile: (data: string, options: { encoding: 'utf8' }) => Promise<unknown>
1115
}
1216

1317
export type ProcessLockFilesystem = {
1418
lstat?: (path: string) => Promise<{ isDirectory: () => boolean; isSymbolicLink: () => boolean; mode: number; uid: number }>
1519
mkdir: (path: string, options: { mode: number; recursive: true }) => Promise<unknown>
16-
open: (path: string, flags: 'wx', mode: number) => Promise<ProcessLockFileHandle>
20+
open: (path: string, flags: number, mode: number) => Promise<ProcessLockFileHandle>
1721
readFile: (path: string, encoding: 'utf8') => Promise<string>
18-
rm: (path: string, options: { force: true }) => Promise<unknown>
22+
tryLock: (fileDescriptor: number) => boolean
23+
}
24+
25+
const nativeFlockCandidates =
26+
process.platform === 'darwin' ? ['/usr/lib/libSystem.B.dylib'] : process.platform === 'linux' ? ['libc.so.6', ...(process.arch === 'x64' ? ['/lib/libc.musl-x86_64.so.1', '/lib/ld-musl-x86_64.so.1'] : []), ...(process.arch === 'arm64' ? ['/lib/libc.musl-aarch64.so.1', '/lib/ld-musl-aarch64.so.1'] : [])] : []
27+
28+
let nativeFlock: ((fileDescriptor: number) => number) | undefined
29+
30+
function tryNativeFileLock(fileDescriptor: number) {
31+
if (nativeFlock === undefined) {
32+
const failures: unknown[] = []
33+
for (const candidate of nativeFlockCandidates) {
34+
try {
35+
const library = dlopen(candidate, { flock: { args: ['i32', 'i32'], returns: 'i32' } })
36+
nativeFlock = descriptor => library.symbols.flock(descriptor, 6)
37+
break
38+
} catch (error) {
39+
failures.push(error)
40+
}
41+
}
42+
if (nativeFlock === undefined) throw new AggregateError(failures, 'Native process locking is unavailable on this platform')
43+
}
44+
return nativeFlock(fileDescriptor) === 0
1945
}
2046

2147
export type ExclusiveProcessLock = {
@@ -28,7 +54,7 @@ const processLockFilesystem: ProcessLockFilesystem = {
2854
mkdir,
2955
open,
3056
readFile,
31-
rm,
57+
tryLock: tryNativeFileLock,
3258
}
3359

3460
async function assertSafeLockDirectory(path: string, filesystem: ProcessLockFilesystem) {
@@ -46,21 +72,34 @@ export async function acquireExclusiveProcessLock(lockPath: string, subject: str
4672
await assertSafeLockDirectory(lockDirectory, filesystem)
4773
let handle: ProcessLockFileHandle
4874
try {
49-
handle = await filesystem.open(lockPath, 'wx', 0o600)
75+
handle = await filesystem.open(lockPath, constants.O_CREAT | constants.O_NOFOLLOW | constants.O_RDWR, 0o600)
5076
} catch (error) {
51-
if (typeof error === 'object' && error !== null && 'code' in error && error.code === 'EEXIST') {
52-
let owner = 'owner metadata unavailable'
53-
try {
54-
owner = (await filesystem.readFile(lockPath, 'utf8')).trim()
55-
} catch (readError) {
56-
void readError
57-
}
58-
throw new Error(`${subject} is already locked (${owner}). Stop the other process before removing ${lockPath}.`)
77+
throw error
78+
}
79+
let acquired: boolean
80+
try {
81+
acquired = filesystem.tryLock(handle.fd)
82+
} catch (error) {
83+
try {
84+
await handle.close()
85+
} catch (cleanupError) {
86+
throw new AggregateError([error, cleanupError], `Failed to acquire and clean up process lock ${lockPath}`)
5987
}
6088
throw error
6189
}
90+
if (!acquired) {
91+
let owner = 'owner metadata unavailable'
92+
try {
93+
owner = (await filesystem.readFile(lockPath, 'utf8')).trim()
94+
} catch (readError) {
95+
void readError
96+
}
97+
await handle.close()
98+
throw new Error(`${subject} is already locked (${owner}). Stop the other process before removing ${lockPath}.`)
99+
}
62100
const payload = `${JSON.stringify({ acquiredAt: new Date().toISOString(), ...metadata, pid: process.pid })}\n`
63101
try {
102+
await handle.truncate(0)
64103
await handle.writeFile(payload, { encoding: 'utf8' })
65104
await handle.chmod(0o600)
66105
await handle.sync()
@@ -71,35 +110,25 @@ export async function acquireExclusiveProcessLock(lockPath: string, subject: str
71110
} catch (cleanupError) {
72111
cleanupErrors.push(cleanupError)
73112
}
74-
try {
75-
await filesystem.rm(lockPath, { force: true })
76-
} catch (cleanupError) {
77-
cleanupErrors.push(cleanupError)
78-
}
79113
if (cleanupErrors.length !== 0) throw new AggregateError([error, ...cleanupErrors], `Failed to initialize and clean up process lock ${lockPath}`)
80114
throw error
81115
}
82116
let released = false
83-
let handleClosed = false
84117
let releaseAttempt: Promise<void> | undefined
85118
return {
86119
path: lockPath,
87120
release: () => {
88121
if (released) return Promise.resolve()
89122
if (releaseAttempt !== undefined) return releaseAttempt
90123
releaseAttempt = (async () => {
91-
if (!handleClosed) {
92-
await handle.close()
93-
handleClosed = true
94-
}
95124
let current: string
96125
try {
97126
current = await filesystem.readFile(lockPath, 'utf8')
98127
} catch (error) {
99128
throw new Error(`Process lock ${lockPath} disappeared before release: ${error instanceof Error ? error.message : String(error)}`)
100129
}
101130
if (current !== payload) throw new Error(`Process lock ${lockPath} changed ownership before release`)
102-
await filesystem.rm(lockPath, { force: true })
131+
await handle.close()
103132
released = true
104133
})().finally(() => {
105134
if (!released) releaseAttempt = undefined

bots/shared/tests/shared-primitives.test.ts

Lines changed: 87 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
import { afterEach, describe, expect, test } from 'bun:test'
2-
import { chmod, mkdtemp, mkdir, open, readFile, rm } from 'node:fs/promises'
2+
import { chmod, mkdtemp, mkdir, open, readFile, rm, stat } from 'node:fs/promises'
33
import { tmpdir } from 'node:os'
4-
import { join } from 'node:path'
4+
import { join, resolve } from 'node:path'
5+
import { pathToFileURL } from 'node:url'
56
import { fetchLogsWithAdaptiveRanges, initialCursor, logRangeLimitError, LogScanError, scanRanges } from '../src/monitoring/block-sync.ts'
67
import { quorumValue, settledQuorumValue } from '../src/monitoring/read-quorum.ts'
78
import { boundedDashboardJson } from '../src/dashboard/security.ts'
@@ -888,8 +889,7 @@ describe('shared bot primitives', () => {
888889
}
889890
})
890891

891-
test('unlinks a failed lock initialization even when closing the handle fails', async () => {
892-
let removed = false
892+
test('reports cleanup failure when failed lock initialization cannot close its handle', async () => {
893893
await expect(
894894
acquireExclusiveProcessLock(
895895
'operator.lock',
@@ -902,41 +902,111 @@ describe('shared bot primitives', () => {
902902
close: async () => {
903903
throw new Error('close failed')
904904
},
905+
fd: 1,
905906
sync: async () => undefined,
907+
truncate: async () => undefined,
906908
writeFile: async () => {
907909
throw new Error('write failed')
908910
},
909911
}),
910912
readFile: async () => '',
911-
rm: async () => {
912-
removed = true
913-
},
913+
tryLock: () => true,
914914
},
915915
),
916916
).rejects.toThrow('Failed to initialize and clean up process lock')
917-
expect(removed).toBe(true)
918917
})
919918

920-
test('retries process-lock cleanup after a transient unlink failure', async () => {
919+
test('closes the file handle when native lock acquisition throws', async () => {
920+
let closed = false
921+
await expect(
922+
acquireExclusiveProcessLock(
923+
'operator.lock',
924+
'Test lock',
925+
{},
926+
{
927+
mkdir: async () => undefined,
928+
open: async () => ({
929+
chmod: async () => undefined,
930+
close: async () => {
931+
closed = true
932+
},
933+
fd: 1,
934+
sync: async () => undefined,
935+
truncate: async () => undefined,
936+
writeFile: async () => undefined,
937+
}),
938+
readFile: async () => '',
939+
tryLock: () => {
940+
throw new Error('native lock unavailable')
941+
},
942+
},
943+
),
944+
).rejects.toThrow('native lock unavailable')
945+
expect(closed).toBe(true)
946+
})
947+
948+
test('retries process-lock cleanup after a transient close failure without replacing the lock inode', async () => {
921949
const directory = await mkdtemp(join(tmpdir(), 'zoltar-process-lock-'))
922950
temporaryDirectories.push(directory)
923951
const lockPath = join(directory, 'operator.lock')
924-
let removals = 0
952+
let closes = 0
925953
const filesystem = {
926954
mkdir,
927955
open,
928956
readFile,
929-
rm: async (path: string, options: { force: true }) => {
930-
removals += 1
931-
if (removals === 1) throw new Error('transient unlink failure')
932-
await rm(path, options)
933-
},
957+
tryLock: () => true,
934958
}
935-
const lock = await acquireExclusiveProcessLock(lockPath, 'Test lock', {}, filesystem)
936-
await expect(lock.release()).rejects.toThrow('transient unlink failure')
959+
const lock = await acquireExclusiveProcessLock(
960+
lockPath,
961+
'Test lock',
962+
{},
963+
{
964+
...filesystem,
965+
open: async (...arguments_) => {
966+
const handle = await open(...arguments_)
967+
return {
968+
chmod: async mode => await handle.chmod(mode),
969+
close: async () => {
970+
closes += 1
971+
if (closes === 1) throw new Error('transient close failure')
972+
await handle.close()
973+
},
974+
fd: handle.fd,
975+
sync: async () => await handle.sync(),
976+
truncate: async length => await handle.truncate(length),
977+
writeFile: async (data, options) => await handle.writeFile(data, options),
978+
}
979+
},
980+
},
981+
)
982+
const inode = (await stat(lockPath)).ino
983+
await expect(lock.release()).rejects.toThrow('transient close failure')
937984
await lock.release()
938985
const replacement = await acquireExclusiveProcessLock(lockPath, 'Test lock', {}, filesystem)
939986
await replacement.release()
987+
expect((await stat(lockPath)).isFile()).toBe(true)
988+
expect((await stat(lockPath)).ino).toBe(inode)
989+
})
990+
991+
test('reclaims a process lock after its owner is killed', async () => {
992+
const directory = await mkdtemp(join(tmpdir(), 'zoltar-killed-process-lock-'))
993+
temporaryDirectories.push(directory)
994+
const lockPath = join(directory, 'operator.lock')
995+
const moduleUrl = pathToFileURL(resolve(import.meta.dir, '../src/execution/process-lock.ts')).href
996+
const child = Bun.spawn([process.execPath, '--eval', `import { acquireExclusiveProcessLock } from ${JSON.stringify(moduleUrl)}; await acquireExclusiveProcessLock(${JSON.stringify(lockPath)}, 'Test lock', {}); console.log('ready'); await Bun.sleep(60_000)`], { stderr: 'pipe', stdout: 'pipe' })
997+
try {
998+
const reader = child.stdout.getReader()
999+
const next = await reader.read()
1000+
if (next.done || !new TextDecoder().decode(next.value).includes('ready')) throw new Error(`Lock subprocess stopped before acquisition: ${await new Response(child.stderr).text()}`)
1001+
reader.releaseLock()
1002+
child.kill('SIGKILL')
1003+
expect(await child.exited).not.toBe(0)
1004+
} finally {
1005+
if (child.exitCode === null) child.kill('SIGKILL')
1006+
await child.exited
1007+
}
1008+
const replacement = await acquireExclusiveProcessLock(lockPath, 'Test lock', {})
1009+
await replacement.release()
9401010
})
9411011

9421012
test('rejects a process-lock directory writable by other users', async () => {

0 commit comments

Comments
 (0)