Repository navigation
fix(embeddings): guard against macOS onnxruntime mutex crash (#68) #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,6 +181,9 @@ | |
| "typescript-eslint": "^8.51.0", | ||
| "vitest": "^4.0.18" | ||
| }, | ||
| "overrides": { | ||
| "@huggingface/transformers>onnxruntime-node": "1.24.3" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new |
||
| }, | ||
| "pnpm": { | ||
| "onlyBuiltDependencies": [ | ||
| "esbuild", | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,65 @@ | ||||||
| import { readFileSync } from 'node:fs'; | ||||||
| import { createRequire } from 'node:module'; | ||||||
| import path from 'node:path'; | ||||||
|
|
||||||
| /** microsoft/onnxruntime#24579 — mutex crash on macOS in 1.21.x and earlier. */ | ||||||
| export const MIN_ONNXRUNTIME_NODE_VERSION = '1.24.0'; | ||||||
|
|
||||||
| const resolve = createRequire(import.meta.url).resolve; | ||||||
|
|
||||||
| export function parseSemverTriple(version: string): [number, number, number] { | ||||||
| const match = /^(\d+)\.(\d+)\.(\d+)/.exec(version.trim()); | ||||||
| if (!match) { | ||||||
| return [0, 0, 0]; | ||||||
| } | ||||||
| return [Number(match[1]), Number(match[2]), Number(match[3])]; | ||||||
| } | ||||||
|
|
||||||
| export function isSemverAtLeast(actual: string, minimum: string): boolean { | ||||||
| const [aMajor, aMinor, aPatch] = parseSemverTriple(actual); | ||||||
| const [mMajor, mMinor, mPatch] = parseSemverTriple(minimum); | ||||||
| if (aMajor !== mMajor) { | ||||||
| return aMajor > mMajor; | ||||||
| } | ||||||
| if (aMinor !== mMinor) { | ||||||
| return aMinor > mMinor; | ||||||
| } | ||||||
| return aPatch >= mPatch; | ||||||
| } | ||||||
|
|
||||||
| function readInstalledOnnxRuntimeVersion(): string | null { | ||||||
| try { | ||||||
| const transformersPkg = resolve('@huggingface/transformers/package.json'); | ||||||
| const onnxPkg = resolve('onnxruntime-node/package.json', { | ||||||
| paths: [path.dirname(transformersPkg)] | ||||||
| }); | ||||||
| const pkg = JSON.parse(readFileSync(onnxPkg, 'utf8')) as { version?: string }; | ||||||
| return typeof pkg.version === 'string' ? pkg.version : null; | ||||||
| } catch { | ||||||
| try { | ||||||
| const onnxPkg = resolve('onnxruntime-node/package.json'); | ||||||
| const pkg = JSON.parse(readFileSync(onnxPkg, 'utf8')) as { version?: string }; | ||||||
| return typeof pkg.version === 'string' ? pkg.version : null; | ||||||
| } catch { | ||||||
| return null; | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| /** Fail fast before ONNX embedding/rerank work when the native runtime is too old. */ | ||||||
| export function assertMinimumOnnxRuntimeNodeVersion(minimum = MIN_ONNXRUNTIME_NODE_VERSION): void { | ||||||
| const installed = readInstalledOnnxRuntimeVersion(); | ||||||
| if (!installed) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| if (isSemverAtLeast(installed, minimum)) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| throw new Error( | ||||||
| `onnxruntime-node@${installed} can crash on macOS during embedding (mutex lock failed; see microsoft/onnxruntime#24579). ` + | ||||||
| `Install @huggingface/transformers@^4.2.0 so onnxruntime-node resolves to >=${minimum}, ` + | ||||||
| `or set EMBEDDING_PROVIDER=openai to bypass local Transformers.js.` | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The shared error recommends
Suggested change
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||
| ); | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| import { readFileSync } from 'node:fs'; | ||
| import { resolve } from 'node:path'; | ||
| import { describe, expect, it } from 'vitest'; | ||
|
|
||
| import { | ||
| MIN_ONNXRUNTIME_NODE_VERSION, | ||
| assertMinimumOnnxRuntimeNodeVersion, | ||
| isSemverAtLeast, | ||
| parseSemverTriple | ||
| } from '../src/utils/onnxruntime-version.js'; | ||
|
|
||
| const root = resolve(import.meta.dirname, '..'); | ||
|
|
||
| type PackageJson = { | ||
| peerDependencies?: Record<string, string>; | ||
| overrides?: Record<string, string>; | ||
| pnpm?: { overrides?: Record<string, string> }; | ||
| }; | ||
|
|
||
| describe('onnxruntime-node macOS mutex regression (#68)', () => { | ||
| const packageJson = JSON.parse(readFileSync(resolve(root, 'package.json'), 'utf8')) as PackageJson; | ||
| const lockfile = readFileSync(resolve(root, 'pnpm-lock.yaml'), 'utf8'); | ||
|
|
||
| it('requires Transformers.js v4 peers (ships onnxruntime-node 1.24.x)', () => { | ||
| expect(packageJson.peerDependencies?.['@huggingface/transformers']).toBe('^4.2.0'); | ||
| }); | ||
|
|
||
| it('pins onnxruntime-node 1.24.3 for npm and pnpm installs', () => { | ||
| expect(packageJson.overrides?.['@huggingface/transformers>onnxruntime-node']).toBe('1.24.3'); | ||
| expect(packageJson.pnpm?.overrides?.['@huggingface/transformers>onnxruntime-node']).toBe('1.24.3'); | ||
| }); | ||
|
|
||
| it('locks onnxruntime-node 1.24.3 in pnpm-lock.yaml', () => { | ||
| expect(lockfile).toContain("'@huggingface/transformers>onnxruntime-node': 1.24.3"); | ||
| expect(lockfile).toMatch(/onnxruntime-node@1\.24\.3/); | ||
| }); | ||
|
|
||
| it('compares semver triples for the minimum safe runtime', () => { | ||
| expect(parseSemverTriple('1.21.0')).toEqual([1, 21, 0]); | ||
| expect(isSemverAtLeast('1.24.3', MIN_ONNXRUNTIME_NODE_VERSION)).toBe(true); | ||
| expect(isSemverAtLeast('1.21.0', MIN_ONNXRUNTIME_NODE_VERSION)).toBe(false); | ||
| }); | ||
|
|
||
| it('accepts onnxruntime-node 1.24.x when installed with Transformers.js', () => { | ||
| expect(() => assertMinimumOnnxRuntimeNodeVersion()).not.toThrow(); | ||
|
Comment on lines
+44
to
+45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The only test that calls |
||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When this checkout is used as the project root, npm rejects the pnpm-style
>selector before resolving dependencies. With npm 11.4.2, bothnpm install --offline --ignore-scripts --package-lock=false --dry-runandnpx --no-install codebase-context memory listfail withOverride without name: @huggingface/transformers>onnxruntime-node. This blocks npm installation and local npx usage, contrary to the root AGENTS.md zero-infra requirement. The bundled npmpackage-jsondocumentation specifies that “Keys can be nested to any arbitrary length”; represent this as an@huggingface/transformersobject containing"onnxruntime-node": "1.24.3"instead.AGENTS.md reference: AGENTS.md:L107-L107
Useful? React with 👍 / 👎.