From 7ad50c2d04ca2df2b5cad35243d6db2d736f676e Mon Sep 17 00:00:00 2001 From: Liam Appelbe Date: Fri, 11 Oct 2024 14:52:36 +1100 Subject: [PATCH] [ffigen] Prepare to publish v15.0.0 --- pkgs/ffigen/CHANGELOG.md | 2 +- pkgs/ffigen/pubspec.yaml | 2 +- pkgs/ffigen/test/native_objc_test/arc_test.m | 1 - .../native_objc_test/block_annotation_test.m | 1 - .../ffigen/test/native_objc_test/block_test.m | 1 - .../native_objc_test/global_native_test.m | 1 - .../test/native_objc_test/global_test.m | 1 - .../test/native_objc_test/isolate_test.m | 1 - .../test/native_objc_test/protocol_test.m | 1 - .../test/native_objc_test/ref_count_test.m | 2 - .../test/native_objc_test/static_func_test.m | 2 - pkgs/ffigen/test/native_objc_test/util.dart | 8 +- pkgs/objective_c/CHANGELOG.md | 2 +- pkgs/objective_c/lib/src/ns_input_stream.dart | 5 + pkgs/objective_c/pubspec.yaml | 2 +- .../test/ns_input_stream_test.dart | 131 ++++++++++++++---- pkgs/objective_c/test/setup.dart | 6 +- .../util.h => objective_c/test/util.c} | 11 +- pkgs/objective_c/test/util.dart | 64 +++++++++ 19 files changed, 191 insertions(+), 53 deletions(-) rename pkgs/{ffigen/test/native_objc_test/util.h => objective_c/test/util.c} (86%) create mode 100644 pkgs/objective_c/test/util.dart diff --git a/pkgs/ffigen/CHANGELOG.md b/pkgs/ffigen/CHANGELOG.md index d1bfb412f..4dcfd01fe 100644 --- a/pkgs/ffigen/CHANGELOG.md +++ b/pkgs/ffigen/CHANGELOG.md @@ -1,4 +1,4 @@ -## 15.0.0-wip +## 15.0.0 - Bump minimum Dart version to 3.4. - Dedupe `ObjCBlock` trampolines to reduce generated ObjC code. diff --git a/pkgs/ffigen/pubspec.yaml b/pkgs/ffigen/pubspec.yaml index cfe73df7a..79a3fbfb5 100644 --- a/pkgs/ffigen/pubspec.yaml +++ b/pkgs/ffigen/pubspec.yaml @@ -3,7 +3,7 @@ # BSD-style license that can be found in the LICENSE file. name: ffigen -version: 15.0.0-wip +version: 15.0.0 description: > Generator for FFI bindings, using LibClang to parse C, Objective-C, and Swift files. diff --git a/pkgs/ffigen/test/native_objc_test/arc_test.m b/pkgs/ffigen/test/native_objc_test/arc_test.m index 291c5e396..858998456 100644 --- a/pkgs/ffigen/test/native_objc_test/arc_test.m +++ b/pkgs/ffigen/test/native_objc_test/arc_test.m @@ -5,7 +5,6 @@ #import #include "arc_test.h" -#include "util.h" #if !__has_feature(objc_arc) #error "This file must be compiled with ARC enabled" diff --git a/pkgs/ffigen/test/native_objc_test/block_annotation_test.m b/pkgs/ffigen/test/native_objc_test/block_annotation_test.m index ba01f43ab..2e7f56cc8 100644 --- a/pkgs/ffigen/test/native_objc_test/block_annotation_test.m +++ b/pkgs/ffigen/test/native_objc_test/block_annotation_test.m @@ -5,7 +5,6 @@ #include #include "block_annotation_test.h" -#include "util.h" @implementation EmptyObject @end diff --git a/pkgs/ffigen/test/native_objc_test/block_test.m b/pkgs/ffigen/test/native_objc_test/block_test.m index 7dbf86de3..ee65f31ab 100644 --- a/pkgs/ffigen/test/native_objc_test/block_test.m +++ b/pkgs/ffigen/test/native_objc_test/block_test.m @@ -7,7 +7,6 @@ #import #include "block_test.h" -#include "util.h" @implementation DummyObject diff --git a/pkgs/ffigen/test/native_objc_test/global_native_test.m b/pkgs/ffigen/test/native_objc_test/global_native_test.m index f126e1467..f63acc4b6 100644 --- a/pkgs/ffigen/test/native_objc_test/global_native_test.m +++ b/pkgs/ffigen/test/native_objc_test/global_native_test.m @@ -5,7 +5,6 @@ #import #include "global_test.h" -#include "util.h" NSString* globalNativeString = @"Hello World"; NSObject* _Nullable globalNativeObject = nil; diff --git a/pkgs/ffigen/test/native_objc_test/global_test.m b/pkgs/ffigen/test/native_objc_test/global_test.m index ec05b20e6..8fb45642f 100644 --- a/pkgs/ffigen/test/native_objc_test/global_test.m +++ b/pkgs/ffigen/test/native_objc_test/global_test.m @@ -5,7 +5,6 @@ #import #include "global_test.h" -#include "util.h" NSString* globalString = @"Hello World"; NSObject* _Nullable globalObject = nil; diff --git a/pkgs/ffigen/test/native_objc_test/isolate_test.m b/pkgs/ffigen/test/native_objc_test/isolate_test.m index 2b8750751..fc205afe6 100644 --- a/pkgs/ffigen/test/native_objc_test/isolate_test.m +++ b/pkgs/ffigen/test/native_objc_test/isolate_test.m @@ -3,7 +3,6 @@ // BSD-style license that can be found in the LICENSE file. #include "isolate_test.h" -#include "util.h" @implementation Sendable @end diff --git a/pkgs/ffigen/test/native_objc_test/protocol_test.m b/pkgs/ffigen/test/native_objc_test/protocol_test.m index c6cc50a5b..c6eaa5618 100644 --- a/pkgs/ffigen/test/native_objc_test/protocol_test.m +++ b/pkgs/ffigen/test/native_objc_test/protocol_test.m @@ -5,7 +5,6 @@ #import #include "protocol_test.h" -#include "util.h" @implementation ProtocolConsumer : NSObject - (NSString*)callInstanceMethod:(id)protocol { diff --git a/pkgs/ffigen/test/native_objc_test/ref_count_test.m b/pkgs/ffigen/test/native_objc_test/ref_count_test.m index 14c54c0d0..38f6d2c01 100644 --- a/pkgs/ffigen/test/native_objc_test/ref_count_test.m +++ b/pkgs/ffigen/test/native_objc_test/ref_count_test.m @@ -5,8 +5,6 @@ #import #import -#include "util.h" - #if __has_feature(objc_arc) #error "This file must be compiled with ARC disabled" #endif diff --git a/pkgs/ffigen/test/native_objc_test/static_func_test.m b/pkgs/ffigen/test/native_objc_test/static_func_test.m index ab46de20d..77964f8da 100644 --- a/pkgs/ffigen/test/native_objc_test/static_func_test.m +++ b/pkgs/ffigen/test/native_objc_test/static_func_test.m @@ -4,8 +4,6 @@ #import -#include "util.h" - void *objc_autoreleasePoolPush(); void objc_autoreleasePoolPop(void *pool); diff --git a/pkgs/ffigen/test/native_objc_test/util.dart b/pkgs/ffigen/test/native_objc_test/util.dart index a40358dea..11d4602e0 100644 --- a/pkgs/ffigen/test/native_objc_test/util.dart +++ b/pkgs/ffigen/test/native_objc_test/util.dart @@ -53,15 +53,15 @@ Future flutterDoGC() async { await Future.delayed(Duration(milliseconds: 500)); } -@Native)>(isLeaf: true, symbol: 'isReadableMemory') -external bool _isReadableMemory(Pointer ptr); +@Native)>(isLeaf: true, symbol: 'isReadableMemory') +external int _isReadableMemory(Pointer ptr); @Native)>( isLeaf: true, symbol: 'getBlockRetainCount') external int _getBlockRetainCount(Pointer block); int blockRetainCount(Pointer block) { - if (!_isReadableMemory(block.cast())) return 0; + if (_isReadableMemory(block.cast()) == 0) return 0; if (!internal_for_testing.isValidBlock(block)) return 0; return _getBlockRetainCount(block.cast()); } @@ -71,7 +71,7 @@ int blockRetainCount(Pointer block) { external int _getObjectRetainCount(Pointer object); int objectRetainCount(Pointer object) { - if (!_isReadableMemory(object.cast())) return 0; + if (_isReadableMemory(object.cast()) == 0) return 0; final header = object.cast().value; // package:objective_c's isValidObject function internally calls diff --git a/pkgs/objective_c/CHANGELOG.md b/pkgs/objective_c/CHANGELOG.md index b5007f54e..54dbb993b 100644 --- a/pkgs/objective_c/CHANGELOG.md +++ b/pkgs/objective_c/CHANGELOG.md @@ -1,4 +1,4 @@ -## 3.0.0-wip +## 3.0.0 - Add the following stream-related types to the core package: - `NSInputStream` diff --git a/pkgs/objective_c/lib/src/ns_input_stream.dart b/pkgs/objective_c/lib/src/ns_input_stream.dart index cc0e3ad5c..068996554 100644 --- a/pkgs/objective_c/lib/src/ns_input_stream.dart +++ b/pkgs/objective_c/lib/src/ns_input_stream.dart @@ -25,22 +25,26 @@ extension NSInputStreamStreamExtension on Stream> { late final StreamSubscription dataSubscription; dataSubscription = listen((data) { + print("NSInputStream: 0"); if (inputStream.addData_(data.toNSData()) > maxReadAheadSize) { dataSubscription.pause(); } }, onError: (Object e) { + print("NSInputStream: 1 $e"); final d = NSMutableDictionary.new1(); d.setObject_forKey_(e.toString().toNSString(), NSLocalizedDescriptionKey); inputStream.setError_(NSError.errorWithDomain_code_userInfo_( 'DartError'.toNSString(), 0, d)); port.close(); }, onDone: () { + print("NSInputStream: 2"); inputStream.setDone(); port.close(); }, cancelOnError: true); dataSubscription.pause(); port.listen((count) { + print("NSInputStream: 3 $count"); // -1 indicates that the `NSInputStream` is closed. All other values // indicate that the `NSInputStream` needs more data. if (count == -1) { @@ -49,6 +53,7 @@ extension NSInputStreamStreamExtension on Stream> { dataSubscription.resume(); } }, onDone: () { + print("NSInputStream: 4"); dataSubscription.cancel(); }); diff --git a/pkgs/objective_c/pubspec.yaml b/pkgs/objective_c/pubspec.yaml index 7ad8c7369..b0aa16333 100644 --- a/pkgs/objective_c/pubspec.yaml +++ b/pkgs/objective_c/pubspec.yaml @@ -4,7 +4,7 @@ name: objective_c description: 'A library to access Objective C from Flutter that acts as a support library for package:ffigen.' -version: 3.0.0-wip +version: 3.0.0 repository: https://github.com/dart-lang/native/tree/main/pkgs/objective_c topics: diff --git a/pkgs/objective_c/test/ns_input_stream_test.dart b/pkgs/objective_c/test/ns_input_stream_test.dart index c9baa02ac..f44885acb 100644 --- a/pkgs/objective_c/test/ns_input_stream_test.dart +++ b/pkgs/objective_c/test/ns_input_stream_test.dart @@ -17,6 +17,8 @@ import 'package:objective_c/objective_c.dart'; import 'package:objective_c/src/objective_c_bindings_generated.dart'; import 'package:test/test.dart'; +import 'util.dart'; + Future<(int, Uint8List, bool, NSStreamStatus, NSError?)> read( NSInputStream stream, int size) async { // TODO(https://github.com/dart-lang/tools/issues/520): @@ -242,38 +244,117 @@ void main() { 'localizedDescription', contains('some exception message')) .having((e) => e.domain.toString(), 'domain', 'DartError')); }); - }); - group('delegate', () { - late DartInputStreamAdapter inputStream; + group('delegate', () { + late DartInputStreamAdapter inputStream; - setUp(() { - inputStream = Stream.fromIterable([ - [1, 2, 3], - ]).toNSInputStream() as DartInputStreamAdapter; - }); + setUp(() { + inputStream = Stream.fromIterable([ + [1, 2, 3], + ]).toNSInputStream() as DartInputStreamAdapter; + }); - test('default delegate', () async { - expect(inputStream.delegate, inputStream); - inputStream.stream_handleEvent_( - inputStream, NSStreamEvent.NSStreamEventOpenCompleted); - }); + test('default delegate', () async { + expect(inputStream.delegate, inputStream); + inputStream.stream_handleEvent_( + inputStream, NSStreamEvent.NSStreamEventOpenCompleted); + }); - test('non-self delegate', () async { - final protoBuilder = ObjCProtocolBuilder(); - final events = []; + test('non-self delegate', () async { + final protoBuilder = ObjCProtocolBuilder(); + final events = []; - NSStreamDelegate.addToBuilder(protoBuilder, - stream_handleEvent_: (stream, event) => events.add(event)); - inputStream.delegate = protoBuilder.build(); - inputStream.stream_handleEvent_( - inputStream, NSStreamEvent.NSStreamEventOpenCompleted); - expect(events, [NSStreamEvent.NSStreamEventOpenCompleted]); + NSStreamDelegate.addToBuilder(protoBuilder, + stream_handleEvent_: (stream, event) => events.add(event)); + inputStream.delegate = protoBuilder.build(); + inputStream.stream_handleEvent_( + inputStream, NSStreamEvent.NSStreamEventOpenCompleted); + expect(events, [NSStreamEvent.NSStreamEventOpenCompleted]); + }); + + test('assign to null', () async { + inputStream.delegate = null; + expect(inputStream.delegate, inputStream); + }); }); - test('assign to null', () async { - inputStream.delegate = null; - expect(inputStream.delegate, inputStream); + group('ref counting', () { + test('with self delegate', () async { + DartInputStreamAdapter? inputStream = Stream.fromIterable([ + [1, 2, 3], + ]).toNSInputStream() as DartInputStreamAdapter; + + expect(inputStream.delegate, inputStream); + + final ptr = inputStream.ref.pointer; + expect(objectRetainCount(ptr), greaterThan(0)); + + inputStream.open(); + inputStream.close(); + inputStream = null; + + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + + expect(objectRetainCount(ptr), 0); + }); + + test('with non-self delegate', () async { + DartInputStreamAdapter? inputStream = Stream.fromIterable([ + [1, 2, 3], + ]).toNSInputStream() as DartInputStreamAdapter; + + inputStream.delegate = NSObject.new1(); + expect(inputStream.delegate, isNot(inputStream)); + + final ptr = inputStream.ref.pointer; + expect(objectRetainCount(ptr), greaterThan(0)); + + inputStream.open(); + while (true) { + final (count, data, hasBytesAvailable, status, error) = + await read(inputStream, 6); + if (count == 0) { + break; + } + } + inputStream = null; + + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + await Future.delayed(Duration.zero); + doGC(); + + expect(objectRetainCount(ptr), 0); + }); }); }); } diff --git a/pkgs/objective_c/test/setup.dart b/pkgs/objective_c/test/setup.dart index 8a215744f..a5e35d894 100644 --- a/pkgs/objective_c/test/setup.dart +++ b/pkgs/objective_c/test/setup.dart @@ -14,7 +14,11 @@ import 'dart:io'; import 'package:args/args.dart'; -const cFiles = ['src/objective_c.c', 'src/include/dart_api_dl.c']; +const cFiles = [ + 'src/objective_c.c', + 'src/include/dart_api_dl.c', + 'test/util.c', +]; const objCFiles = [ 'src/input_stream_adapter.m', 'src/objective_c.m', diff --git a/pkgs/ffigen/test/native_objc_test/util.h b/pkgs/objective_c/test/util.c similarity index 86% rename from pkgs/ffigen/test/native_objc_test/util.h rename to pkgs/objective_c/test/util.c index f9a67e81b..09c6016bd 100644 --- a/pkgs/ffigen/test/native_objc_test/util.h +++ b/pkgs/objective_c/test/util.c @@ -1,10 +1,7 @@ -// Copyright (c) 2023, the Dart project authors. Please see the AUTHORS file +// 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. -#ifndef _TEST_UTIL_H_ -#define _TEST_UTIL_H_ - #include #include @@ -33,7 +30,7 @@ uint64_t getObjectRetainCount(ObjectRefCountExtractor* object) { return count < 0x80 ? count : k128OrMore; } -bool isReadableMemory(void* ptr) { +int isReadableMemory(void* ptr) { vm_map_t task = mach_task_self(); mach_vm_address_t address = (mach_vm_address_t)ptr; mach_vm_size_t size = 0; @@ -43,9 +40,7 @@ bool isReadableMemory(void* ptr) { kern_return_t status = mach_vm_region(task, &address, &size, VM_REGION_BASIC_INFO_64, (vm_region_info_t)&info, &count, &object_name); - if (status != KERN_SUCCESS) return false; + if (status != KERN_SUCCESS) return 0; return ((mach_vm_address_t)ptr) >= address && (info.protection & VM_PROT_READ); } - -#endif // _TEST_UTIL_H_ diff --git a/pkgs/objective_c/test/util.dart b/pkgs/objective_c/test/util.dart new file mode 100644 index 000000000..4ad7649cd --- /dev/null +++ b/pkgs/objective_c/test/util.dart @@ -0,0 +1,64 @@ +// 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. + +// TODO: Should we share this with ffigen and move it to an unpublished util +// package in this repo? + +import 'dart:ffi'; +import 'dart:io'; + +import 'package:ffi/ffi.dart'; +import 'package:objective_c/objective_c.dart'; +import 'package:objective_c/src/internal.dart' as internal_for_testing + show isValidClass; + +final _executeInternalCommand = () { + try { + return DynamicLibrary.process() + .lookup, Pointer)>>( + 'Dart_ExecuteInternalCommand') + .asFunction, Pointer)>(); + } on ArgumentError { + return null; + } +}(); + +bool canDoGC = _executeInternalCommand != null; + +void doGC() { + final gcNow = 'gc-now'.toNativeUtf8(); + _executeInternalCommand!(gcNow.cast(), nullptr); + calloc.free(gcNow); +} + +@Native)>(isLeaf: true, symbol: 'isReadableMemory') +external int _isReadableMemory(Pointer ptr); + +@Native)>( + isLeaf: true, symbol: 'getObjectRetainCount') +external int _getObjectRetainCount(Pointer object); + +int objectRetainCount(Pointer object) { + if (_isReadableMemory(object.cast()) == 0) return 0; + final header = object.cast().value; + + // package:objective_c's isValidObject function internally calls + // object_getClass then isValidClass. But object_getClass can occasionally + // crash for invalid objects. This masking logic is a simplified version of + // what object_getClass does internally. This is less likely to crash, but + // more likely to break due to ObjC runtime updates, which is a reasonable + // trade off to make in tests where we're explicitly calling it many times + // on invalid objects. In package:objective_c's case, it doesn't matter so + // much if isValidObject crashes, since it's a best effort attempt to give a + // nice stack trace before the real crash, but it would be a problem if + // isValidObject broke due to a runtime update. + // These constants are the ISA_MASK macro defined in runtime/objc-private.h. + const maskX64 = 0x00007ffffffffff8; + const maskArm = 0x00000001fffffff8; + final mask = Abi.current() == Abi.macosX64 ? maskX64 : maskArm; + final clazz = Pointer.fromAddress(header & mask); + + if (!internal_for_testing.isValidClass(clazz)) return 0; + return _getObjectRetainCount(object.cast()); +}