huggingface / huggingface/candle

MetalDevice::new panics on empty MTLCopyAllDevices() instead of returning Err

Open Beginner friendly
#3,566 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
21k
Forks
1.8k
Avg merge
16h 42m
Merged PRs (30d)
25

Description

### Bug

`MetalDevice::new(ordinal)` in `candle-core/src/metal_backend/mod.rs:1873`
calls `metal::Device::all().swap_remove(ordinal)` unconditionally. When
`MTLCopyAllDevices()` returns an empty `Vec` (which is the documented
behaviour in several legitimate macOS contexts, see below), `swap_remove(0)`
panics on an empty `Vec`:

```
thread 'tokio-runtime-worker' panicked at library/alloc/src/vec/mod.rs:1981:13:
swap_remove index (is 0) should be < len (is 0)
```

Because this is a `panic!` and not a returned `Err`, downstream code that
does

```rust
if let Ok(device) = Device::new_metal(0) {
// use Metal
} else {
// fall back to CPU
}
```

cannot recover — the panic unwinds the stack past the `if let` and tears
the process down. The only workaround today is `std::panic::catch_unwind`
on the caller side, which shouldn't be required.

### Repro

`candle-core = "0.9"` (also reproduces on `main`).

```rust
use candle_core::Device;

fn main() {
let _ = Device::new_metal(0); // panics instead of returning Err
}
```

Triggers on any macOS environment where `MTLCopyAllDevices()` returns an
empty array. Known triggers:

* Apple Silicon process started from a non-GUI session (SSH, launchd
daemon without `launchctl asuser`, detached tmux/screen launched from
a different console) — Metal requires WindowServer association.
* Quarantined / unsigned binary distributed via GitHub Releases or
similar — `com.apple.quarantine` xattr can suppress Metal enumeration.
* macOS Sonoma 14.1.x where Metal device enumeration was flaky right
after `screensaver`/`wake` (regression fixed in 14.4+).
* Headless / sandboxed contexts (containers, CI runners without a
display server).

Confirmed in the wild on macOS Sonoma 14.1.2 + M1 (16 GB) — the binary
panics at the `Device::new_metal(0)` call and the user can only recover
by forcing CPU via an env var on the caller side.

### Expected

`MetalDevice::new` should return `Err(MetalError::NoDevice)` (or any
existing error variant) when `Device::all()` is empty, so that callers
can handle the "no Metal GPU available" case via the normal `Result`
contract.

### Proposed fix

```rust
fn new(ordinal: usize) -> Result {
let mut devices = Device::all();
if ordinal >= devices.len() {
return Err(MetalError::Message(format!(
"no Metal device at ordinal {ordinal} (MTLCopyAllDevices returned {} devices)",
devices.len()
)).into());
}
let device = devices.swap_remove(ordinal);
// ... rest unchanged
}
```

Happy to send a PR if the approach (returning `Err` rather than `panic!`)
is acceptable to maintainers.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start at candle-core/src/metal_backend/mod.rs:1873 and inspect MetalDevice::new, Device::all(), and the existing MetalError variants. Confirm that an empty device list or an out-of-range ordinal returns an Err rather than panicking; the reproducer in the issue should then complete without unwinding.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend, operating-systems
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
84/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.