From 6805f437594da827287d86ab37ad0bcd65c82c78 Mon Sep 17 00:00:00 2001 From: tannevaled Date: Tue, 8 Sep 2026 16:11:11 +0200 Subject: [PATCH] DispatchMain gives the block back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every call leaked one Objective-C block and one entry in purego's block table -- the entry that holds the Go closure alive -- for the life of the process. Nothing ever removed either: purego's dispose helper is what deletes a table entry, and dispose only runs when the last reference to the block goes. Measured, one process posting blocks and draining them on the main thread: 3 576 blocks 14.2 MB resident 42 824 blocks 22.3 MB 161 782 blocks 44.4 MB about 200 bytes a call, never returned. After: 3 296 blocks 13.5 MB 39 328 blocks 17.8 MB 157 096 blocks 19.7 MB Growth over the same span falls from +30.2 MB to +6.2 MB. An application that hops to the main thread once a frame reached the old numbers in minutes. Releasing here is safe because dispatch_async takes its own reference, and must: its contract is that the caller may free the block as soon as the call returns. ⛔ THE TEST'S SEAM HAD TO BE COMPLETED, NOT JUST USED. The scheduled-hop test fakes mkBlock with a sentinel; leaving the real rmBlock in place sent 0xB10C to _Block_release and the suite died with "fault 0xb114". A seam that is only half replaced runs half the real thing. ⚠ NOT a fix for the crash this was found while chasing. go-xrkit/desk's suite faults inside cfRunLoopRunInMode at iokit hid_darwin.go:434, invoking a block that belongs here; with this change it still does, 2 runs in 10 against 4 in 10 before, which is one sample inside another's noise. That is a separate defect and is filed as one. Co-Authored-By: Claude Opus 5 --- dispatch_darwin.go | 25 ++++++++++++++++++++++++- objc_darwin_test.go | 20 +++++++++++++++++--- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/dispatch_darwin.go b/dispatch_darwin.go index 3f5b8e0..69c0e44 100644 --- a/dispatch_darwin.go +++ b/dispatch_darwin.go @@ -36,6 +36,9 @@ var ( mkBlock = func(fn func()) uintptr { return uintptr(objc.NewBlock(func(objc.Block) { fn() })) } + // rmBlock drops OUR reference to a block once dispatch_async has taken its + // own. See [DispatchMain]. + rmBlock = func(b uintptr) { objc.Block(b).Release() } ) // loadDispatch resolves dispatch_async and the main queue from libSystem, @@ -62,6 +65,24 @@ func loadDispatch() { // methods must be messaged on the main thread). A nil fn is a no-op. If // libdispatch cannot be resolved, fn runs inline on the calling goroutine rather // than being dropped. +// +// ⛔⛔ AND THE BLOCK IS RELEASED. It was not, and every call leaked one for the +// life of the process — the block itself, and the entry purego keeps in its +// block table to hold the Go closure alive. Measured, one process posting +// blocks and draining them on the main thread: +// +// 3 576 blocks 14.2 MB resident +// 42 824 blocks 22.3 MB +// 161 782 blocks 44.4 MB +// +// which is about 200 bytes a call, never returned. An application that hops to +// the main thread once a frame reaches those numbers in minutes. +// +// ⭐ RELEASING HERE IS SAFE BECAUSE dispatch_async TAKES ITS OWN REFERENCE, and +// must: its contract is that the caller may free the block as soon as the call +// returns. So the count goes to two and back to one here, libdispatch drops the +// last one after running the block, and purego's dispose helper then removes +// the table entry — which is the only thing that ever removes one. func DispatchMain(fn func()) { if fn == nil { return @@ -71,5 +92,7 @@ func DispatchMain(fn func()) { fn() return } - dispatchAsyncFn(dispatchMainQ, mkBlock(fn)) + b := mkBlock(fn) + dispatchAsyncFn(dispatchMainQ, b) + rmBlock(b) } diff --git a/objc_darwin_test.go b/objc_darwin_test.go index 2659d02..a69be3a 100644 --- a/objc_darwin_test.go +++ b/objc_darwin_test.go @@ -268,12 +268,12 @@ func TestOnDevice_DispatchMainScheduledHop(t *testing.T) { if dispatchLoadErr != nil || dispatchMainQ == 0 { t.Fatalf("real load failed: err=%v q=%#x", dispatchLoadErr, dispatchMainQ) } - savedAsync, savedBlock := dispatchAsyncFn, mkBlock - defer func() { dispatchAsyncFn, mkBlock = savedAsync, savedBlock }() + savedAsync, savedBlock, savedRm := dispatchAsyncFn, mkBlock, rmBlock + defer func() { dispatchAsyncFn, mkBlock, rmBlock = savedAsync, savedBlock, savedRm }() const sentinel uintptr = 0xB10C var captured func() - var gotQ, gotBlock uintptr + var gotQ, gotBlock, freed uintptr mkBlock = func(fn func()) uintptr { captured = fn; return sentinel } dispatchAsyncFn = func(q, block uintptr) { gotQ, gotBlock = q, block @@ -281,6 +281,11 @@ func TestOnDevice_DispatchMainScheduledHop(t *testing.T) { captured() } } + // ⛔ AND rmBlock IS FAKED TOO, or the sentinel goes to _Block_release: this + // test crashed with "fault 0xb114" -- 0xB10C plus the offset a release reads + // -- the moment the real one was left in place. A seam that is only half + // replaced is a seam that runs half the real thing. + rmBlock = func(b uintptr) { freed = b } ran := false DispatchMain(func() { ran = true }) @@ -293,6 +298,15 @@ func TestOnDevice_DispatchMainScheduledHop(t *testing.T) { if !ran { t.Fatal("DispatchMain did not run the scheduled fn") } + // ⛔⛔ AND THE BLOCK IS GIVEN BACK. Without this, every call leaked a block + // and the purego table entry holding its closure: measured at about 200 + // bytes a call, 3 576 blocks costing 14.2 MB resident against 44.4 MB for + // 161 782. dispatch_async takes its own reference, so releasing ours here is + // what its contract is for. + if freed != sentinel { + t.Errorf("the block was released as %#x, want %#x: DispatchMain is "+ + "leaking one block and one closure per call", freed, sentinel) + } t.Log("on-device: DispatchMain scheduled fn onto the main queue via dispatch_async") }