From 996ff3969f8a5bbc1fb24b3d9c510b57bab3f52c Mon Sep 17 00:00:00 2001 From: Andrey Kolkov Date: Sat, 1 Aug 2026 12:08:04 +0300 Subject: [PATCH] =?UTF-8?q?fix(metal):=20eliminate=20unsafe.Pointer=20in?= =?UTF-8?q?=20block=20callbacks=20=E2=80=94=20map=20lookup=20(#293)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- CHANGELOG.md | 13 +++++++++ hal/metal/objc.go | 67 ++++++++++++++++++++++++++++++++++++----------- 2 files changed, 64 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 231db64..208c758 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,19 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.30.31] - 2026-08-01 + +### Fixed + +- **Metal: block callbacks crash under `-race` (checkptr)** — all 4 ObjC block + callback trampolines converted `uintptr blockPtr` to `unsafe.Pointer` to read + `blockID` at offset 32. checkptr rejects this because `uintptr` from goffi's + reflect callback has no pointer provenance. Replaced with reverse map lookup + (`blockPtrToID sync.Map`) — block pointer used as opaque integer key only, + no `unsafe.Pointer` conversion. Follows purego (Ebitengine) pattern. + wgpu#280 fix was insufficient — fixed arithmetic but not the base conversion. + (#293) + ## [0.30.30] - 2026-07-31 ### Fixed diff --git a/hal/metal/objc.go b/hal/metal/objc.go index f39f2d0..305e954 100644 --- a/hal/metal/objc.go +++ b/hal/metal/objc.go @@ -587,6 +587,13 @@ var blockRegistry sync.Map // map[uint64]*blockRegistryEntry // collect the block before Metal invokes the callback. var blockPinRegistry sync.Map // map[uint64]*blockLiteral +// blockPtrToID maps block pointer (uintptr) → blockID (uint64). +// Reverse lookup used by callbacks to recover the blockID without converting +// the uintptr to unsafe.Pointer — avoids checkptr violation under -race. +// Follows purego pattern: block pointer used as opaque integer key only. +// See issue #293. +var blockPtrToID sync.Map // map[uintptr]uint64 + // blockIDCounter is the next block ID to assign. Atomically incremented. var blockIDCounter uint64 @@ -646,9 +653,13 @@ func getSharedEventBlockInvoke() uintptr { if blockPtr == 0 { return } - // Read blockID from the block literal at the fixed offset. - // Offset: isa(8) + flags(4) + reserved(4) + invoke(8) + descriptor(8) = 32 bytes - blockID := *(*uint64)(unsafe.Add(unsafe.Pointer(blockPtr), 32)) + // Look up blockID via reverse map — no unsafe.Pointer conversion. + // Avoids checkptr violation under -race (purego pattern, #293). + val, ok := blockPtrToID.Load(blockPtr) + if !ok { + return + } + blockID := val.(uint64) hal.Logger().Debug("metal: shared event notification fired", "blockID", blockID) @@ -698,14 +709,20 @@ func newSharedEventNotificationBlock() (uintptr, uint64, chan struct{}) { } // Pin the block so GC doesn't collect it before the callback fires. + blockPtr := uintptr(unsafe.Pointer(block)) blockPinRegistry.Store(id, block) + blockPtrToID.Store(blockPtr, id) - return uintptr(unsafe.Pointer(block)), id, done + return blockPtr, id, done } // releaseBlock removes the block entry from the registry and unpins the block. // Must be called after the block fires or times out to prevent memory leaks. func releaseBlock(id uint64) { + if pinned, ok := blockPinRegistry.Load(id); ok { + block := pinned.(*blockLiteral) + blockPtrToID.Delete(uintptr(unsafe.Pointer(block))) + } blockRegistry.Delete(id) blockPinRegistry.Delete(id) } @@ -749,9 +766,13 @@ func getCompletedHandlerBlockInvoke() uintptr { if blockPtr == 0 { return 0 } - // Read blockID from the block literal at the fixed offset. - // Offset: isa(8) + flags(4) + reserved(4) + invoke(8) + descriptor(8) = 32 bytes - blockID := *(*uint64)(unsafe.Add(unsafe.Pointer(blockPtr), 32)) + // Look up blockID via reverse map — no unsafe.Pointer conversion. + // Avoids checkptr violation under -race (purego pattern, #293). + val, ok := blockPtrToID.Load(blockPtr) + if !ok { + return 0 + } + blockID := val.(uint64) hal.Logger().Debug("metal: completion handler fired", "blockID", blockID) @@ -804,9 +825,11 @@ func newCompletedHandlerBlock(stagingBuffer ID) uintptr { } // Pin the block so GC doesn't collect it before the callback fires. + blockPtr := uintptr(unsafe.Pointer(block)) blockPinRegistry.Store(id, block) + blockPtrToID.Store(blockPtr, id) - return uintptr(unsafe.Pointer(block)) + return blockPtr } // -------------------------------------------------------------------------- @@ -845,9 +868,13 @@ func getFrameCompletionBlockInvoke() uintptr { if blockPtr == 0 { return 0 } - // Read blockID from the block literal at the fixed offset. - // Offset: isa(8) + flags(4) + reserved(4) + invoke(8) + descriptor(8) = 32 bytes - blockID := *(*uint64)(unsafe.Add(unsafe.Pointer(blockPtr), 32)) + // Look up blockID via reverse map — no unsafe.Pointer conversion. + // Avoids checkptr violation under -race (purego pattern, #293). + val, ok := blockPtrToID.Load(blockPtr) + if !ok { + return 0 + } + blockID := val.(uint64) hal.Logger().Debug("metal: frame completion fired", "blockID", blockID) @@ -902,9 +929,11 @@ func newFrameCompletionBlock(frameSemaphore chan struct{}) uintptr { } // Pin the block so GC doesn't collect it before the callback fires. + blockPtr := uintptr(unsafe.Pointer(block)) blockPinRegistry.Store(id, block) + blockPtrToID.Store(blockPtr, id) - return uintptr(unsafe.Pointer(block)) + return blockPtr } // -------------------------------------------------------------------------- @@ -953,9 +982,13 @@ func getGPUCompletionBlockInvoke() uintptr { if blockPtr == 0 { return 0 } - // Read blockID from the block literal at the fixed offset. - // Offset: isa(8) + flags(4) + reserved(4) + invoke(8) + descriptor(8) = 32 bytes - blockID := *(*uint64)(unsafe.Add(unsafe.Pointer(blockPtr), 32)) + // Look up blockID via reverse map — no unsafe.Pointer conversion. + // Avoids checkptr violation under -race (purego pattern, #293). + val, ok := blockPtrToID.Load(blockPtr) + if !ok { + return 0 + } + blockID := val.(uint64) hal.Logger().Debug("metal: GPU completion tracking fired", "blockID", blockID) @@ -1020,7 +1053,9 @@ func newGPUCompletionBlock(target *atomic.Uint64, submissionIndex uint64) uintpt } // Pin the block so GC doesn't collect it before the callback fires. + blockPtr := uintptr(unsafe.Pointer(block)) blockPinRegistry.Store(id, block) + blockPtrToID.Store(blockPtr, id) - return uintptr(unsafe.Pointer(block)) + return blockPtr }