Skip to content

fix(engine): mtlBuf's documented copy fallback was an infinite recursion - #178

Merged
penta2himajin merged 1 commit into
mainfrom
claude/issue177-mtlbuf-recursion
Aug 2, 2026
Merged

fix(engine): mtlBuf's documented copy fallback was an infinite recursion#178
penta2himajin merged 1 commit into
mainfrom
claude/issue177-mtlbuf-recursion

Conversation

@penta2himajin

Copy link
Copy Markdown
Owner

Closes #177.

if let b = a.asMTLBuffer(device: device, noCopy: true) { return b }
return mtlBuf(a, device)          // same arguments -> infinite recursion

The docstring promises a copy fallback ("不可なら copy"). There was none: the fallback called itself with identical arguments, so a nil from the no-copy wrap would recurse until the stack overflowed. It now passes noCopy: false — what the comment already described.

Latent, not live

mlx-swift currently copies internally rather than returning nil, so today the branch is unreachable. It is latent across 267 call sites, and whether it stays unreachable is a property of mlx-swift's implementation, not of qwisp. A version that returns nil instead of copying would turn every weight-buffer wrap into a crash rather than a slow path.

No test, deliberately

The branch cannot be reached without forcing asMTLBuffer to return nil, and there is no hook for that. A test that only exercises the happy path would raise the locked-test count without guarding anything — the opposite of what that counter is for.

Found while

Attributing the 8GB tier's memory. nonMLXmetal is metal - active - cache with metal = MTLDevice.currentAllocatedSize, and mtlBuf wrappers alias existing pages: they inflate that counter without costing resident memory. This repo had already documented exactly that in #150 (TellRuntime.swift:640-644) — reading mtlBuf to confirm it was a view and not a copy is what surfaced the recursion.

Gates

RAWTESTS 100/100 · BENCHBATCHTEST PASS · COMPTEST 97/97 · CBGUARD PASS

🤖 Generated with Claude Code

…ion (#177)

    if let b = a.asMTLBuffer(device: device, noCopy: true) { return b }
    return mtlBuf(a, device)          // same arguments

The docstring promises "不可なら copy". There was no copy path — the fallback
called itself with identical arguments, so a nil from the no-copy wrap would
recurse until the stack overflowed. It now passes noCopy: false, which is what
the comment already described.

Latent, not live: mlx-swift currently copies internally rather than returning
nil, so the branch is unreachable today. It is latent across 267 call sites
though, and whether it stays unreachable is a property of mlx-swift's
implementation, not of qwisp — a version that returns nil instead of copying
would turn every weight-buffer wrap into a crash.

No test: the branch cannot be reached without forcing asMTLBuffer to return nil,
and there is no hook for that. A test that only exercises the happy path would
raise the locked-test count without guarding anything.

Found while attributing the 8GB tier's memory. nonMLXmetal is metal - active -
cache with metal = MTLDevice.currentAllocatedSize, and mtlBuf wrappers alias
existing pages — they inflate that counter without costing resident memory,
which this repo already documented in #150 (TellRuntime.swift:640-644). Reading
mtlBuf to confirm it was a view and not a copy is what surfaced the recursion.

Gates: RAWTESTS 100/100, BENCHBATCHTEST PASS, COMPTEST 97/97, CBGUARD PASS.

Closes #177

Co-Authored-By: Claude <noreply@anthropic.com>
@penta2himajin
penta2himajin merged commit a0dfbac into main Aug 2, 2026
1 check passed
@penta2himajin
penta2himajin deleted the claude/issue177-mtlbuf-recursion branch August 2, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mtlBuf's documented copy fallback is an infinite recursion

1 participant