fix(metal): eliminate unsafe.Pointer in block callbacks — map lookup (#293) - #294
Conversation
…293) All 4 ObjC block callbacks converted uintptr blockPtr to unsafe.Pointer to dereference blockID at offset 32. checkptr rejects this under -race because uintptr from goffi reflect callback has no pointer provenance. Replace with blockPtrToID sync.Map reverse lookup — block pointer used as opaque integer key only. No unsafe.Pointer conversion. Follows purego (Ebitengine) pattern. wgpu#280 fix was insufficient. Fixes #293.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
in case this proves helpful in further debugging: i noticed that the TestDarwinAppRunSmoke test, when executed in isolation from other tests on my MacBook Air M1, opens an application window (an application named "darwin.text" with a main window titled "gogpu") and the test will hang indefinitely, until a human user triggers a device input event (e.g., a keyboard key pressed, or a movement of the mouse pointer) to reproduce in a clean workspace: $ cat go.mod go 1.25.0 require ( require ( $ go test -run ^TestDarwinAppRunSmoke$ ./internal/platform/darwin # test will hang until the system receives device input |
|
further investigation needed: $ GOTOOLCHAIN=go1.25.0 go test -race ./internal/platform/darwin goroutine 45 gp=0xc0000036c0 m=4 mp=0xc000057808 [running]: goroutine 1 gp=0xc0000021c0 m=nil [chan receive, locked to thread]: goroutine 18 gp=0xc00008a380 m=nil [force gc (idle)]: goroutine 19 gp=0xc00008a540 m=nil [GC sweep wait]: goroutine 20 gp=0xc00008a700 m=nil [GC scavenge wait]: goroutine 21 gp=0xc00008a8c0 m=nil [GOMAXPROCS updater (idle)]: goroutine 22 gp=0xc00008ae00 m=nil [finalizer wait]: goroutine 23 gp=0xc00008afc0 m=nil [chan receive]: |
|
Thanks for the detailed reports, @jbunds. Your first checkptr crash (block callbacks) is fixed in this PR (v0.30.31) — all 4 ObjC block trampolines now use a map lookup instead of The second crash ( We reported it: go-webgpu/goffi#67 Already fixed and released: goffi v0.6.3 — https://github.com/go-webgpu/goffi/releases/tag/v0.6.3 We'll bump wgpu's goffi dep to v0.6.3 in the next patch. After that, Regarding |
|
I've just sync'ed my workspace with HEAD on upstream main: $ git --no-pager log -n 1 --format='%H %d' HEAD upstream/main $ git diff upstream/main..HEAD # null output $ go clean -testcache $ GOTOOLCHAIN=go1.25.0 go test -race ./internal/platform/darwin goroutine 1 gp=0xc0000021c0 m=0 mp=0x104f1db40 [running, locked to thread]: goroutine 2 gp=0xc000002c40 m=nil [force gc (idle)]: goroutine 3 gp=0xc000003180 m=nil [GC sweep wait]: goroutine 4 gp=0xc000003340 m=nil [sleep]: goroutine 5 gp=0xc000003880 m=nil [GOMAXPROCS updater (idle)]: goroutine 6 gp=0xc000003dc0 m=nil [finalizer wait]: goroutine 7 gp=0xc00015e000 m=nil [chan receive]: goroutine 54 gp=0xc000082700 m=nil [chan receive]: goroutine 10 gp=0xc00015e1c0 m=nil [GC worker (idle)]: goroutine 11 gp=0xc00015e8c0 m=nil [GC worker (idle)]: goroutine 32 gp=0xc000083180 m=nil [GC worker (idle)]: goroutine 33 gp=0xc000342540 m=nil [GC worker (idle)]: goroutine 51 gp=0xc000342700 m=nil [GC worker (idle)]: goroutine 52 gp=0xc0003428c0 m=nil [GC worker (idle)]: goroutine 53 gp=0xc000342a80 m=nil [GC worker (idle)]: goroutine 12 gp=0xc00015ea80 m=nil [GC worker (idle)]: |
Summary
All 4 ObjC block callback trampolines converted
uintptr blockPtrtounsafe.Pointerto read blockID. checkptr under-racerejects this — uintptr from goffi has no pointer provenance. wgpu#280 fix (unsafe.Add) was insufficient — base conversion itself invalid.Replace with
blockPtrToID sync.Mapreverse lookup (purego/Ebitengine pattern). Block pointer used as opaque integer key. Zero unsafe.Pointer conversions in callbacks.Test plan
unsafe.Pointer(blockPtr)in callbacksFixes #293.