Skip to content

Commit 692c2da

Browse files
authored
[objective_c] Fix a bug where toNSInputStream would keep listening to events unless NSInputStream.close is called (#2392)
1 parent cc5539f commit 692c2da

File tree

9 files changed

+266
-6
lines changed

9 files changed

+266
-6
lines changed

pkgs/ffigen/lib/src/code_generator/objc_built_in_types.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
const objCBuiltInInterfaces = {
88
'DOBJCDartInputStreamAdapter': 'DartInputStreamAdapter',
9+
'DOBJCDartInputStreamAdapterWeakHolder': 'DartInputStreamAdapterWeakHolder',
910
'DOBJCObservation': 'DOBJCObservation',
1011
'DOBJCDartProtocolBuilder': 'DartProtocolBuilder',
1112
'DOBJCDartProtocol': 'DartProtocol',

pkgs/objective_c/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
- Bump minimum Dart version to 3.8.0.
44
- Support the KVO pattern by adding `Observer`, `Observation`, and
55
`NSObject.addObserver`.
6+
- Remove a reference cycle between Dart and the `NSInputStream` returned by
7+
`toNSInputStream`.
68

79
## 8.0.0
810

pkgs/objective_c/ffigen_objc.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ objc-interfaces:
2727
# Keep in sync with ffigen's ObjCBuiltInFunctions.builtInInterfaces.
2828
include:
2929
- DOBJCDartInputStreamAdapter
30+
- DOBJCDartInputStreamAdapterWeakHolder
3031
- DOBJCObservation
3132
- DOBJCDartProtocolBuilder
3233
- DOBJCDartProtocol
@@ -72,6 +73,7 @@ objc-interfaces:
7273
- Protocol
7374
rename:
7475
'DOBJCDartInputStreamAdapter': 'DartInputStreamAdapter'
76+
'DOBJCDartInputStreamAdapterWeakHolder' : 'DartInputStreamAdapterWeakHolder'
7577
'DOBJCDartProtocolBuilder': 'DartProtocolBuilder'
7678
'DOBJCDartProtocol': 'DartProtocol'
7779
objc-protocols:

pkgs/objective_c/lib/src/ns_input_stream.dart

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,22 @@ import 'dart:async';
22
import 'dart:ffi';
33
import 'dart:isolate';
44

5-
import 'ns_data.dart';
6-
import 'ns_string.dart';
5+
import '../objective_c.dart';
6+
77
import 'objective_c_bindings_generated.dart';
88

9+
@Native<Pointer<Void> Function()>(
10+
isLeaf: true,
11+
symbol: 'objc_autoreleasePoolPush',
12+
)
13+
external Pointer<Void> _autoreleasePoolPush();
14+
15+
@Native<Void Function(Pointer<Void>)>(
16+
isLeaf: true,
17+
symbol: 'objc_autoreleasePoolPop',
18+
)
19+
external void _autoreleasePoolPop(Pointer<Void> pool);
20+
921
extension NSInputStreamStreamExtension on Stream<List<int>> {
1022
/// Return a [NSInputStream] that, when read, will contain the contents of
1123
/// the [Stream].
@@ -15,23 +27,57 @@ extension NSInputStreamStreamExtension on Stream<List<int>> {
1527
/// > than the one that calls [toNSInputStream]. Otherwise,
1628
/// > [NSInputStream.read] will deadlock waiting for data to be added from the
1729
/// > [Stream].
30+
///
31+
/// > [!IMPORTANT]
32+
/// > [NSInputStream.read] must be called from a different thread or [Isolate]
33+
/// > than the one that calls `toNSInputStream`. Otherwise,
34+
/// > [NSInputStream.read] will deadlock waiting for data to be added from the
35+
/// > [Stream].
36+
///
37+
/// > [!IMPORTANT]
38+
/// > `toNSInputStream` creates a reference cycle between Dart and
39+
/// > Objective-C. Unless this cycle is broken, the [Isolate] calling
40+
/// > `toNSInputStream` will never exit. The cycle can be broken by calling
41+
/// > [NSInputStream.close] or releasing the `NSInputStream` using
42+
/// > `NSInputStream.ref.release()`.
1843
NSInputStream toNSInputStream() {
1944
// Eagerly add data until `maxReadAheadSize` is buffered.
2045
const maxReadAheadSize = 4096;
2146

2247
final port = ReceivePort();
23-
final inputStream = DartInputStreamAdapter.inputStreamWithPort(
24-
port.sendPort.nativePort,
25-
);
48+
49+
final DartInputStreamAdapter inputStream;
50+
final DartInputStreamAdapterWeakHolder weakInputStream;
51+
52+
// Only hold a weak reference to the returned `inputStream` so that there is
53+
// no unbreakable reference cycle between Dart and Objective-C. When the
54+
// `inputStream`'s `dealloc` method is called then it sends this code a
55+
// message saying that it was closed.
56+
final pool = _autoreleasePoolPush();
57+
try {
58+
inputStream = DartInputStreamAdapter.inputStreamWithPort(
59+
port.sendPort.nativePort,
60+
);
61+
weakInputStream =
62+
DartInputStreamAdapterWeakHolder.holderWithInputStreamAdapter(
63+
inputStream,
64+
);
65+
} finally {
66+
_autoreleasePoolPop(pool);
67+
}
68+
2669
late final StreamSubscription<dynamic> dataSubscription;
2770

2871
dataSubscription = listen(
2972
(data) {
73+
final inputStream = weakInputStream.adapter;
3074
if (inputStream.addData(data.toNSData()) > maxReadAheadSize) {
3175
dataSubscription.pause();
3276
}
77+
inputStream.ref.release();
3378
},
3479
onError: (Object e) {
80+
final inputStream = weakInputStream.adapter;
3581
final d = NSMutableDictionary();
3682
d[NSLocalizedDescriptionKey] = e.toString().toNSString();
3783
inputStream.setError(
@@ -41,10 +87,13 @@ extension NSInputStreamStreamExtension on Stream<List<int>> {
4187
userInfo: d,
4288
),
4389
);
90+
inputStream.ref.release();
4491
port.close();
4592
},
4693
onDone: () {
94+
final inputStream = weakInputStream.adapter;
4795
inputStream.setDone();
96+
inputStream.ref.release();
4897
port.close();
4998
},
5099
cancelOnError: true,

pkgs/objective_c/lib/src/objective_c_bindings_generated.dart

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,6 +1129,154 @@ class DartInputStreamAdapter extends NSInputStream implements NSStreamDelegate {
11291129
}
11301130
}
11311131

1132+
/// Helper class that contains a weak reference to a `DOBJCDartInputStreamAdapter`.
1133+
class DartInputStreamAdapterWeakHolder extends NSObject {
1134+
DartInputStreamAdapterWeakHolder._(
1135+
ffi.Pointer<objc.ObjCObject> pointer, {
1136+
bool retain = false,
1137+
bool release = false,
1138+
}) : super.castFromPointer(pointer, retain: retain, release: release);
1139+
1140+
/// Constructs a [DartInputStreamAdapterWeakHolder] that points to the same underlying object as [other].
1141+
DartInputStreamAdapterWeakHolder.castFrom(objc.ObjCObjectBase other)
1142+
: this._(other.ref.pointer, retain: true, release: true);
1143+
1144+
/// Constructs a [DartInputStreamAdapterWeakHolder] that wraps the given raw object pointer.
1145+
DartInputStreamAdapterWeakHolder.castFromPointer(
1146+
ffi.Pointer<objc.ObjCObject> other, {
1147+
bool retain = false,
1148+
bool release = false,
1149+
}) : this._(other, retain: retain, release: release);
1150+
1151+
/// Returns whether [obj] is an instance of [DartInputStreamAdapterWeakHolder].
1152+
static bool isInstance(objc.ObjCObjectBase obj) {
1153+
return _objc_msgSend_19nvye5(
1154+
obj.ref.pointer,
1155+
_sel_isKindOfClass_,
1156+
_class_DOBJCDartInputStreamAdapterWeakHolder,
1157+
);
1158+
}
1159+
1160+
/// alloc
1161+
static DartInputStreamAdapterWeakHolder alloc() {
1162+
final _ret = _objc_msgSend_151sglz(
1163+
_class_DOBJCDartInputStreamAdapterWeakHolder,
1164+
_sel_alloc,
1165+
);
1166+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1167+
_ret,
1168+
retain: false,
1169+
release: true,
1170+
);
1171+
}
1172+
1173+
/// allocWithZone:
1174+
static DartInputStreamAdapterWeakHolder allocWithZone(
1175+
ffi.Pointer<NSZone> zone,
1176+
) {
1177+
final _ret = _objc_msgSend_1cwp428(
1178+
_class_DOBJCDartInputStreamAdapterWeakHolder,
1179+
_sel_allocWithZone_,
1180+
zone,
1181+
);
1182+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1183+
_ret,
1184+
retain: false,
1185+
release: true,
1186+
);
1187+
}
1188+
1189+
/// holderWithInputStreamAdapter:
1190+
static DartInputStreamAdapterWeakHolder holderWithInputStreamAdapter(
1191+
DartInputStreamAdapter adapter,
1192+
) {
1193+
final _ret = _objc_msgSend_1sotr3r(
1194+
_class_DOBJCDartInputStreamAdapterWeakHolder,
1195+
_sel_holderWithInputStreamAdapter_,
1196+
adapter.ref.pointer,
1197+
);
1198+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1199+
_ret,
1200+
retain: true,
1201+
release: true,
1202+
);
1203+
}
1204+
1205+
/// new
1206+
static DartInputStreamAdapterWeakHolder new$() {
1207+
final _ret = _objc_msgSend_151sglz(
1208+
_class_DOBJCDartInputStreamAdapterWeakHolder,
1209+
_sel_new,
1210+
);
1211+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1212+
_ret,
1213+
retain: false,
1214+
release: true,
1215+
);
1216+
}
1217+
1218+
/// adapter
1219+
DartInputStreamAdapter get adapter {
1220+
final _ret = _objc_msgSend_151sglz(this.ref.pointer, _sel_adapter);
1221+
return DartInputStreamAdapter.castFromPointer(
1222+
_ret,
1223+
retain: true,
1224+
release: true,
1225+
);
1226+
}
1227+
1228+
/// autorelease
1229+
DartInputStreamAdapterWeakHolder autorelease() {
1230+
final _ret = _objc_msgSend_151sglz(this.ref.pointer, _sel_autorelease);
1231+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1232+
_ret,
1233+
retain: true,
1234+
release: true,
1235+
);
1236+
}
1237+
1238+
/// init
1239+
DartInputStreamAdapterWeakHolder init() {
1240+
objc.checkOsVersionInternal(
1241+
'DOBJCDartInputStreamAdapterWeakHolder.init',
1242+
iOS: (false, (2, 0, 0)),
1243+
macOS: (false, (10, 0, 0)),
1244+
);
1245+
final _ret = _objc_msgSend_151sglz(
1246+
this.ref.retainAndReturnPointer(),
1247+
_sel_init,
1248+
);
1249+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1250+
_ret,
1251+
retain: false,
1252+
release: true,
1253+
);
1254+
}
1255+
1256+
/// retain
1257+
DartInputStreamAdapterWeakHolder retain() {
1258+
final _ret = _objc_msgSend_151sglz(this.ref.pointer, _sel_retain);
1259+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1260+
_ret,
1261+
retain: true,
1262+
release: true,
1263+
);
1264+
}
1265+
1266+
/// self
1267+
DartInputStreamAdapterWeakHolder self$1() {
1268+
final _ret = _objc_msgSend_151sglz(this.ref.pointer, _sel_self);
1269+
return DartInputStreamAdapterWeakHolder.castFromPointer(
1270+
_ret,
1271+
retain: true,
1272+
release: true,
1273+
);
1274+
}
1275+
1276+
/// Returns a new instance of DartInputStreamAdapterWeakHolder constructed with the default `new` method.
1277+
factory DartInputStreamAdapterWeakHolder() => new$();
1278+
}
1279+
11321280
/// Base class of all classes DOBJCDartProtocolBuilder creates.
11331281
class DartProtocol extends NSObject {
11341282
DartProtocol._(
@@ -34547,6 +34695,9 @@ class Protocol extends objc.ObjCObjectBase {
3454734695
late final _class_DOBJCDartInputStreamAdapter = objc.getClass(
3454834696
"DOBJCDartInputStreamAdapter",
3454934697
);
34698+
late final _class_DOBJCDartInputStreamAdapterWeakHolder = objc.getClass(
34699+
"DOBJCDartInputStreamAdapterWeakHolder",
34700+
);
3455034701
late final _class_DOBJCDartProtocol = objc.getClass("DOBJCDartProtocol");
3455134702
late final _class_DOBJCDartProtocolBuilder = objc.getClass(
3455234703
"DOBJCDartProtocolBuilder",
@@ -38828,6 +38979,7 @@ late final _sel_absoluteURLWithDataRepresentation_relativeToURL_ = objc
3882838979
late final _sel_acceptInputForMode_beforeDate_ = objc.registerName(
3882938980
"acceptInputForMode:beforeDate:",
3883038981
);
38982+
late final _sel_adapter = objc.registerName("adapter");
3883138983
late final _sel_addChild_withPendingUnitCount_ = objc.registerName(
3883238984
"addChild:withPendingUnitCount:",
3883338985
);
@@ -39255,6 +39407,9 @@ late final _sel_hasSpaceAvailable = objc.registerName("hasSpaceAvailable");
3925539407
late final _sel_hasSuffix_ = objc.registerName("hasSuffix:");
3925639408
late final _sel_hash = objc.registerName("hash");
3925739409
late final _sel_helpAnchor = objc.registerName("helpAnchor");
39410+
late final _sel_holderWithInputStreamAdapter_ = objc.registerName(
39411+
"holderWithInputStreamAdapter:",
39412+
);
3925839413
late final _sel_host = objc.registerName("host");
3925939414
late final _sel_illegalCharacterSet = objc.registerName("illegalCharacterSet");
3926039415
late final _sel_implementMethod_withBlock_withTrampoline_withSignature_ = objc

pkgs/objective_c/src/input_stream_adapter.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,11 @@
2020
- (void)setDone;
2121
- (void)setError:(NSError *)error;
2222
@end
23+
24+
/// Helper class that contains a weak reference to a `DOBJCDartInputStreamAdapter`.
25+
@interface DOBJCDartInputStreamAdapterWeakHolder : NSObject
26+
27+
+ (instancetype)holderWithInputStreamAdapter:(DOBJCDartInputStreamAdapter *)adapter;
28+
@property(nonatomic, readonly, weak) DOBJCDartInputStreamAdapter* adapter;
29+
30+
@end

pkgs/objective_c/src/input_stream_adapter.m

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ + (instancetype)inputStreamWithPort:(Dart_Port)sendPort {
3333
return stream;
3434
}
3535

36+
- (void)dealloc {
37+
if (_status != NSStreamStatusClosed) {
38+
Dart_PostInteger_DL(_sendPort, -1);
39+
}
40+
}
41+
3642
- (NSUInteger)addData:(NSData *)data {
3743
[_dataCondition lock];
3844
[_data appendData:data];
@@ -167,3 +173,16 @@ - (void)stream:(NSStream *)theStream handleEvent:(NSStreamEvent)streamEvent {
167173
}
168174

169175
@end
176+
177+
@implementation DOBJCDartInputStreamAdapterWeakHolder
178+
+ (instancetype)holderWithInputStreamAdapter:(DOBJCDartInputStreamAdapter *)adapter {
179+
DOBJCDartInputStreamAdapterWeakHolder * holder = [[DOBJCDartInputStreamAdapterWeakHolder alloc] init];
180+
if (holder != nil) {
181+
holder->adapter = adapter;
182+
}
183+
return holder;
184+
}
185+
186+
@synthesize adapter;
187+
188+
@end

0 commit comments

Comments
 (0)