huggingface / huggingface/candle
MetalDevice::new panics on empty MTLCopyAllDevices() instead of returning Err
- 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