-
Notifications
You must be signed in to change notification settings - Fork 126
[objective_c] Fix GC safepoint crash in ObjCProtocolBuilder.implementMethod #3307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
liamappelbe
merged 15 commits into
dart-lang:main
from
Henawey:DOBJCDartProtocolBuilder-crash-fix
Apr 28, 2026
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
5056cb4
[objective_c] Fix GC safepoint crash in ObjCProtocolBuilder.implement…
Henawey 75c7063
[objective_c] Enhance finalizable_test with runtime GC assertions
Henawey 5b52b41
[objective_c] Scope Finalizable to ObjCBlockBase only (not _ObjCRefHo…
Henawey d41be54
[objective_c] Add deterministic GC-safepoint regression test for issu…
Henawey d7ee271
[objective_c] Switch issue #3209 fix to blockRef extraction approach
Henawey 54b8752
[objective_c] Clean up: remove desymbolize.py, fix CHANGELOG wording
Henawey 3e3a901
[objective_c] Trim docs: remove REPRODUCTION.md, reduce comments, sho…
Henawey 2fb7c93
[objective_c] Rename finalizable_test → gc_safepoint_test, note nativ…
Henawey ad804db
[objective_c] Fix and clean up gc_safepoint_test
Henawey 7df6c14
[objective_c] Fix protocol object GC liveness in gc_safepoint_test
Henawey 54e772a
[objective_c] Fix block retain count checks to use util.c
Henawey 1ba6b4f
[objective_c] Fix protocol object GC test: use never-inline sync helper
Henawey 47d6a58
[objective_c] dart format test/util.dart
Henawey 4d3343f
[objective_c] Rename gc_safepoint_test → protocol_builder_test, remov…
Henawey 1757d86
Merge branch 'main' into DOBJCDartProtocolBuilder-crash-fix
liamappelbe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| // Copyright (c) 2026, the Dart project authors. Please see the AUTHORS file | ||
| // for details. All rights reserved. Use of this source code is governed by a | ||
| // BSD-style license that can be found in the LICENSE file. | ||
|
|
||
| // GC injection helpers for regression-testing issue #3209. | ||
| // | ||
| // Swizzles DOBJCDartProtocolBuilder's implementMethod:withBlock:... to inject | ||
| // a Dart GC at the FFI safepoint between Dart extracting the raw block pointer | ||
| // and Objective-C retaining it — the exact window where premature GC can | ||
| // release the block before the handover completes. | ||
| // | ||
| // Run AOT to reproduce under production-like conditions — GC liveness issues | ||
| // like this are unreliable in JIT (from pkgs/objective_c/). | ||
| // Native assets must be enabled; stable from Dart 3.10.0: | ||
| // dart compile exe test/gc_safepoint_test.dart -o /tmp/gc_test | ||
| // DYLD_INSERT_LIBRARIES=.dart_tool/lib/objective_c.dylib /tmp/gc_test | ||
|
|
||
| #import <Foundation/Foundation.h> | ||
| #import <objc/runtime.h> | ||
| #include <dlfcn.h> | ||
| #include <stdbool.h> | ||
| #include <stdint.h> | ||
|
|
||
| // From util.c — reads the block ABI flags field for the retain count. | ||
| extern uint64_t getBlockRetainCount(void*); | ||
|
|
||
| typedef void* (*DartGCNow_t)(const char*, void*); | ||
| static DartGCNow_t g_dart_gc_now = NULL; | ||
|
|
||
| static IMP g_original_imp = NULL; | ||
| static bool g_swizzle_active = false; | ||
| // Written and read from the same Dart thread (native-mode FFI call), so a | ||
| // plain bool is safe. | ||
| static bool g_block_freed_before_retain = false; | ||
|
|
||
| // Replacement for -[DOBJCDartProtocolBuilder implementMethod:withBlock:...]. | ||
| // Forces GC before calling the original, then checks the retain count. | ||
| static void gc_inject_imp( | ||
| id self, SEL _cmd, SEL sel, void* block, | ||
| void* trampoline, const char* signature) { | ||
| if (g_swizzle_active && g_dart_gc_now != NULL) { | ||
| g_dart_gc_now("gc-now", NULL); | ||
| // Use the same util as the Dart side (util.c:getBlockRetainCount). | ||
| int count = (int)getBlockRetainCount(block); | ||
| if (count == 0) { | ||
| g_block_freed_before_retain = true; | ||
| } | ||
| } | ||
| ((void (*)(id, SEL, SEL, void*, void*, const char*))g_original_imp)( | ||
| self, _cmd, sel, block, trampoline, signature); | ||
| } | ||
|
|
||
| // Look up Dart_ExecuteInternalCommand via dlsym. Must be called once before | ||
| // installing the swizzle. | ||
| void initGCInject(void) { | ||
| g_dart_gc_now = | ||
| (DartGCNow_t)dlsym(RTLD_DEFAULT, "Dart_ExecuteInternalCommand"); | ||
| } | ||
|
|
||
| bool gcNowAvailableFromNative(void) { | ||
| return g_dart_gc_now != NULL; | ||
| } | ||
|
|
||
| // Triggers gc-now from inside a non-leaf FFI call (Dart thread is at a | ||
| // safepoint), used to verify GC can actually fire from native mode. | ||
| void callGCNowFromNative(void) { | ||
| if (g_dart_gc_now != NULL) { | ||
| g_dart_gc_now("gc-now", NULL); | ||
| } | ||
| } | ||
|
|
||
| void installGCInjectSwizzle(void) { | ||
| g_block_freed_before_retain = false; | ||
| Class cls = NSClassFromString(@"DOBJCDartProtocolBuilder"); | ||
| if (cls == nil) return; | ||
| SEL sel = | ||
| @selector(implementMethod:withBlock:withTrampoline:withSignature:); | ||
| Method m = class_getInstanceMethod(cls, sel); | ||
| if (m == NULL) return; | ||
| g_original_imp = method_setImplementation(m, (IMP)gc_inject_imp); | ||
| } | ||
|
|
||
| void removeGCInjectSwizzle(void) { | ||
| if (g_original_imp == NULL) return; | ||
| Class cls = NSClassFromString(@"DOBJCDartProtocolBuilder"); | ||
| SEL sel = | ||
| @selector(implementMethod:withBlock:withTrampoline:withSignature:); | ||
| Method m = class_getInstanceMethod(cls, sel); | ||
| if (m != NULL) { | ||
| method_setImplementation(m, g_original_imp); | ||
| } | ||
| g_original_imp = NULL; | ||
| } | ||
|
|
||
| void setGCInjectActive(bool active) { | ||
| g_swizzle_active = active; | ||
| } | ||
|
|
||
| // Sticky flag: once true it stays true even after setGCInjectActive(false). | ||
| bool wasBlockFreedBeforeRetain(void) { | ||
| return g_block_freed_before_retain; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,119 @@ | ||
| // Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file | ||
| // for details. All rights reserved. Use of this source code is governed by a | ||
| // BSD-style license that can be found in the LICENSE file. | ||
|
|
||
| // Objective C support is only available on mac. | ||
| @TestOn('mac-os') | ||
| library; | ||
|
|
||
| import 'package:objective_c/objective_c.dart'; | ||
| import 'package:objective_c/src/objective_c_bindings_generated.dart' | ||
| show ObjCBlock_ffiVoid_ffiVoid_NSStream_NSStreamEvent; | ||
| import 'package:test/test.dart'; | ||
|
|
||
| import 'util.dart'; | ||
|
|
||
| // Directly exercises the blockRef extraction pattern from the production fix. | ||
| // Extract block.ref into a local `blockRef` (static type ObjCBlockRef, which | ||
| // transitively implements Finalizable). The Finalizable contract keeps blockRef | ||
| // live in the GC stack map across the non-leaf FFI safepoint below. | ||
| // | ||
| // Intentionally never-inlined so the JIT performs its own liveness analysis. | ||
| @pragma('vm:never-inline') | ||
| bool _gcAndCheckBlock() { | ||
| final block = ObjCBlock_ffiVoid_ffiVoid_NSStream_NSStreamEvent.fromFunction( | ||
| (_, stream, event) {}, | ||
| keepIsolateAlive: false, | ||
| ); | ||
| final blockRef = block.ref; | ||
| final ptr = blockRef.pointer; | ||
| // Use callGCNowFromNative (non-leaf safepoint) to trigger GC, then check | ||
| // the retain count via blockRetainCount, which reads the block ABI flags | ||
| // field — the correct location for block retain counts on all architectures. | ||
| callGCNowFromNative(); | ||
| return blockRetainCount(ptr) > 0; | ||
| } | ||
|
|
||
| void main() { | ||
| group('block wrapper not freed at GC safepoints', () { | ||
| setUpAll(() { | ||
| initGCInject(); | ||
| installGCInjectSwizzle(); | ||
| }); | ||
|
|
||
| tearDownAll(removeGCInjectSwizzle); | ||
|
|
||
| // Diagnostic: verify that gc-now from native code actually triggers GC. | ||
| // If this test fails, the GC-injection swizzle is a no-op and the | ||
| // reproduction test below is not meaningful. | ||
| test('gc-now from native code collects unreachable objects', () { | ||
| if (!canDoGC) { | ||
| markTestSkipped( | ||
| 'Dart_ExecuteInternalCommand unavailable — GC injection is a no-op.', | ||
| ); | ||
| return; | ||
| } | ||
| expect(gcNowAvailableFromNative(), isTrue); | ||
|
|
||
| WeakReference<Object>? weakRef; | ||
| (() { | ||
| final obj = Object(); | ||
| weakRef = WeakReference(obj); | ||
| })(); | ||
|
|
||
| callGCNowFromNative(); | ||
|
|
||
| expect(weakRef!.target, isNull); | ||
| }); | ||
|
|
||
| // Swizzle injects gc-now before ObjC retains the block. Without the | ||
| // blockRef extraction fix, the optimizer marks `block` dead after its raw | ||
| // pointer is extracted and GC drops the retain count to 0. | ||
| // Run 1000 iterations to trigger JIT optimisation of implementMethod. | ||
| test('block survives GC injected inside implementMethod ' | ||
| '(fails without blockRef extraction)', () { | ||
| const kIterations = 1000; | ||
| for (var i = 0; i < kIterations; i++) { | ||
| final builder = ObjCProtocolBuilder(); | ||
| setGCInjectActive(true); | ||
| NSStreamDelegate$Builder.stream_handleEvent_.implement( | ||
| builder, | ||
| (stream, event) {}, | ||
| ); | ||
| setGCInjectActive(false); | ||
| // wasBlockFreedBeforeRetain() is sticky: stays true once set. | ||
| if (wasBlockFreedBeforeRetain()) break; | ||
| } | ||
|
|
||
| expect( | ||
| wasBlockFreedBeforeRetain(), | ||
| isFalse, | ||
| reason: | ||
| 'Block was prematurely released by GC before ObjC retained it. ' | ||
| 'blockRef extraction in implementMethod is required (issue #3209).', | ||
| ); | ||
| }); | ||
|
|
||
| test('block local NOT freed at non-leaf FFI safepoint', () { | ||
| // Guaranteed to reproduce on iteration 1 with: | ||
| // dart --optimization-counter-threshold=0 test ... | ||
| if (!canDoGC) { | ||
| markTestSkipped( | ||
| 'Dart_ExecuteInternalCommand unavailable — gc-now is a no-op, ' | ||
| 'test would pass vacuously.', | ||
| ); | ||
| return; | ||
| } | ||
| const kIterations = 1000; | ||
| for (var i = 0; i < kIterations; i++) { | ||
| final survived = _gcAndCheckBlock(); | ||
| if (!survived) { | ||
| fail( | ||
| 'Block wrapper was GC-collected at FFI safepoint on iteration $i. ' | ||
| 'blockRef extraction in implementMethod required (issue #3209).', | ||
| ); | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.