-
Notifications
You must be signed in to change notification settings - Fork 60
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
[ffigen] Allow blocks and protocols to keep their isolates alive #2017
Conversation
PR HealthBreaking changes ✔️
Changelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs.
API leaks
|
Package | Leaked API symbols |
---|---|
objective_c | _Version |
This check can be disabled by tagging the PR with skip-leaking-check
.
License Headers ✔️
// Copyright (c) 2025, 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.
Files |
---|
no missing headers |
All source files should start with a license header.
Unrelated files missing license headers
Files |
---|
pkgs/jni/lib/src/third_party/generated_bindings.dart |
pkgs/objective_c/lib/src/ns_input_stream.dart |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep the APIs similar:
In JNIgen I use one port per implemented interface, I don't store the port anywhere (should I?) but it does reference itself and when it receives a message from Java it closes itself:
(_$impls
is a map from native ports to the implementation object that contains the Dart functions)
late final jni$_.RawReceivePort $p;
$p = jni$_.RawReceivePort(($m) {
if ($m == null) {
_$impls.remove($p.sendPort.nativePort);
$p.close();
return;
}
final $i = jni$_.MethodInvocation.fromMessage($m);
final $r = _$invokeMethod($p.sendPort.nativePort, $i);
jni$_.ProtectedJniExtensions.returnResult($i.result, $r);
});
If I want to add keepIsolateAlive
, do I have to simply set it for $p
like $p.keepIsolateAlive = keepIsolateAlive
?
We also need to add the mechanism we talked about with the finalizers so the objc/java side knows that the callback is no longer valid.
/// If `keepIsolateAlive` is true, this block will keep this isolate alive | ||
/// until it is garbage collected by both Dart and ObjC. | ||
static $blockType listener(${func.dartType} fn, | ||
{bool keepIsolateAlive = false}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it safer to have this default to true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a breaking change, so I was avoiding it. But thinking about it again, you're right, it should default to true.
Yeah, that's how I implemented it for protocols too.
Yep, I think that's probably all you need.
Yeah, it's on my list. |
Add a
keepIsolateAlive
parameter to the block constructors. When true, this creates aRawReceivePort
and stores it adjacent to the closure in the block closure registry. When the block is GC'd, the port is closed.Similarly, add a
keepIsolateAlive
parameter to the protocol constructor. This is also implemented using aRawReceivePort
, but in this case the port holds a reference to itself, and closes itself when it receives any message. That message is sent byDOBJCDartProxy
'sdispose
method.Fixes #447