diff --git a/pkgs/objective_c/CHANGELOG.md b/pkgs/objective_c/CHANGELOG.md index 66ccb0bae8..cf382a5a30 100644 --- a/pkgs/objective_c/CHANGELOG.md +++ b/pkgs/objective_c/CHANGELOG.md @@ -1,3 +1,13 @@ +## 9.4.2-wip + +- Add a deterministic regression test for the GC-safepoint bug in + `DartProtocolBuilder.buildInstance` (same family as + [#3209](https://github.com/dart-lang/native/issues/3209)). The bug itself is + structurally prevented by the ffigen change in + [#3352](https://github.com/dart-lang/native/pull/3352) (which extracts + `.ref.pointer` into a Finalizable local before every FFI call); this test + guards against future regressions of that pattern. + ## 9.4.1 - Fix a [bug](https://github.com/flutter/flutter/issues/186794) related to diff --git a/pkgs/objective_c/pubspec.yaml b/pkgs/objective_c/pubspec.yaml index 6e4e38901e..5e8823d107 100644 --- a/pkgs/objective_c/pubspec.yaml +++ b/pkgs/objective_c/pubspec.yaml @@ -3,7 +3,7 @@ # BSD-style license that can be found in the LICENSE file. name: objective_c description: 'A library to access Objective C from Flutter that acts as a support library for package:ffigen.' -version: 9.4.1 +version: 9.4.2-wip repository: https://github.com/dart-lang/native/tree/main/pkgs/objective_c issue_tracker: https://github.com/dart-lang/native/issues?q=is%3Aissue+is%3Aopen+label%3Apackage%3Aobjective_c diff --git a/pkgs/objective_c/test/gc_inject.m b/pkgs/objective_c/test/gc_inject.m index d5169b2b72..375687e9eb 100644 --- a/pkgs/objective_c/test/gc_inject.m +++ b/pkgs/objective_c/test/gc_inject.m @@ -31,6 +31,19 @@ uint64_t getBlockRetainCount(BlockRefCountExtractor* block) { return (block->flags & 0xFFFF) >> 1; } +// On arm64 non-pointer ISA, the top byte of the isa header holds extra_rc +// (i.e. retain count minus one). This is sufficient to observe a retain-count +// drop, which is all the buildInstance swizzle below needs to detect a +// premature finalizer release. +typedef struct { + uint64_t header; +} ObjectRefCountExtractor; + +__attribute__((visibility("default"))) +uint64_t getObjectRetainCount(ObjectRefCountExtractor* object) { + return object->header >> 56; +} + typedef void* (*DartGCNow_t)(const char*, void*); static DartGCNow_t g_dart_gc_now = NULL; @@ -40,6 +53,11 @@ uint64_t getBlockRetainCount(BlockRefCountExtractor* block) { // plain bool is safe. static bool g_block_freed_before_retain = false; +// buildInstance swizzle state — separate from the implementMethod swizzle +// above so both can be installed independently. Both share g_swizzle_active. +static IMP g_original_buildInstance_imp = NULL; +static bool g_builder_released_during_buildInstance = false; + // Replacement for -[DOBJCDartProtocolBuilder implementMethod:withBlock:...]. // Forces GC before calling the original, then checks the retain count. static void gc_inject_imp( @@ -107,3 +125,61 @@ void setGCInjectActive(bool active) { bool wasBlockFreedBeforeRetain(void) { return g_block_freed_before_retain; } + +// Replacement for -[DOBJCDartProtocolBuilder buildInstance:]. The production +// crash (same family as #3209) is the Dart wrapper for the receiver being +// finalized at the FFI safepoint inside the call, after Dart extracts +// _builder.ref.pointer but before objc_msgSend dispatches. This swizzle +// reproduces that window deterministically: +// +// 1. CFRetain self so the test process survives even if the finalizer fires +// — without this, we'd observe the crash instead of detecting it. +// 2. Snapshot retain count, force gc-now, snapshot again. A drop means a +// finalizer released the builder while we were inside the FFI call. +// 3. Call the original IMP and return its result. +// 4. CFRelease to balance the retain from step 1. +static id buildInstance_gc_inject_imp(id self, SEL _cmd, int32_t port) { + if (g_swizzle_active && g_dart_gc_now != NULL) { + CFRetain((CFTypeRef)self); + uint64_t rc_before = getObjectRetainCount((__bridge void*)self); + g_dart_gc_now("gc-now", NULL); + uint64_t rc_after = getObjectRetainCount((__bridge void*)self); + if (rc_after < rc_before) { + g_builder_released_during_buildInstance = true; + } + id result = + ((id (*)(id, SEL, int32_t))g_original_buildInstance_imp)( + self, _cmd, port); + CFRelease((CFTypeRef)self); + return result; + } + return ((id (*)(id, SEL, int32_t))g_original_buildInstance_imp)( + self, _cmd, port); +} + +void installBuildInstanceSwizzle(void) { + g_builder_released_during_buildInstance = false; + Class cls = NSClassFromString(@"DOBJCDartProtocolBuilder"); + if (cls == nil) return; + SEL sel = @selector(buildInstance:); + Method m = class_getInstanceMethod(cls, sel); + if (m == NULL) return; + g_original_buildInstance_imp = + method_setImplementation(m, (IMP)buildInstance_gc_inject_imp); +} + +void removeBuildInstanceSwizzle(void) { + if (g_original_buildInstance_imp == NULL) return; + Class cls = NSClassFromString(@"DOBJCDartProtocolBuilder"); + SEL sel = @selector(buildInstance:); + Method m = class_getInstanceMethod(cls, sel); + if (m != NULL) { + method_setImplementation(m, g_original_buildInstance_imp); + } + g_original_buildInstance_imp = NULL; +} + +// Sticky flag: once true it stays true even after setGCInjectActive(false). +bool wasBuilderReleasedDuringBuildInstance(void) { + return g_builder_released_during_buildInstance; +} diff --git a/pkgs/objective_c/test/protocol_builder_test.dart b/pkgs/objective_c/test/protocol_builder_test.dart index a335293723..5a0840250a 100644 --- a/pkgs/objective_c/test/protocol_builder_test.dart +++ b/pkgs/objective_c/test/protocol_builder_test.dart @@ -124,4 +124,67 @@ void main() { } }); }); + + group('builder NOT released during buildInstance ' + '(regression for production crash, same family as #3209)', () { + setUpAll(() { + initGCInject(); + installBuildInstanceSwizzle(); + }); + + tearDownAll(removeBuildInstanceSwizzle); + + // Swizzle injects gc-now inside buildInstance:. Without ffigen extracting + // the receiver's .ref into a Finalizable local before the FFI call (see + // https://github.com/dart-lang/native/pull/3352), the AOT optimizer can + // mark the wrapper dead immediately after pointer extraction; if GC runs + // at the safepoint, the wrapper's Finalizer calls objc_release on the + // builder, dealloc tombstones the ISA, and objc_msgSend crashes on the + // now-dangling receiver. The swizzle uses CFRetain/CFRelease around the + // gc-now so we observe the cause (a retain-count drop) instead of the + // crash itself. + // + // Reproduces deterministically in AOT (`dart test -c exe`) on iteration 0 + // when the ffigen extraction is reverted. In JIT the bug is non- + // deterministic, but the test still runs as a smoke check that the + // swizzle infrastructure is wired up and no regression is observable + // under 1000 iterations. + test('builder survives GC injected inside buildInstance ' + '(fails in AOT without ffigen #3352 ref extraction)', () { + if (!canDoGC) { + markTestSkipped( + 'Dart_ExecuteInternalCommand unavailable — GC injection is a no-op.', + ); + return; + } + const kIterations = 1000; + for (var i = 0; i < kIterations; i++) { + final builder = ObjCProtocolBuilder(); + // Drain transient wrappers created inside _createBuilder() (e.g. the + // alloc() result that's a separate Dart wrapper for the same ObjC + // object) before we enter the measurement window. Otherwise their + // Finalizers can fire during the swizzled gc-now and report as a + // release that isn't actually the receiver of buildInstance:. + doGC(); + setGCInjectActive(true); + final instance = builder.build(keepIsolateAlive: false); + setGCInjectActive(false); + // Touch instance so the optimizer cannot dead-store the whole call. + if (instance.ref.pointer == nullptr) { + fail('buildInstance returned nullptr on iteration $i.'); + } + // wasBuilderReleasedDuringBuildInstance is sticky: stays true once set. + if (wasBuilderReleasedDuringBuildInstance()) break; + } + + expect( + wasBuilderReleasedDuringBuildInstance(), + isFalse, + reason: + 'Builder was released by Dart finalizer during buildInstance ' + 'call. The receiver-side .ref extraction generated by ffigen ' + 'is required (analogous to issue #3209).', + ); + }); + }); } diff --git a/pkgs/objective_c/test/util.dart b/pkgs/objective_c/test/util.dart index 0cad521534..8f97e9b49b 100644 --- a/pkgs/objective_c/test/util.dart +++ b/pkgs/objective_c/test/util.dart @@ -93,3 +93,18 @@ external bool gcNowAvailableFromNative(); // requires the Dart thread to be at a proper native-mode safepoint. @Native(symbol: 'callGCNowFromNative') external void callGCNowFromNative(); + +// buildInstance swizzle controls — mirror the implementMethod-side trio above. +// Same `setGCInjectActive` flag gates both swizzles uniformly. + +@Native(isLeaf: true, symbol: 'installBuildInstanceSwizzle') +external void installBuildInstanceSwizzle(); + +@Native(isLeaf: true, symbol: 'removeBuildInstanceSwizzle') +external void removeBuildInstanceSwizzle(); + +@Native( + isLeaf: true, + symbol: 'wasBuilderReleasedDuringBuildInstance', +) +external bool wasBuilderReleasedDuringBuildInstance();