[ffigen] Migrate callbacks to Dart_CObject_kNativePointer#3458
[ffigen] Migrate callbacks to Dart_CObject_kNativePointer#3458mertalev wants to merge 7 commits into
Dart_CObject_kNativePointer#3458Conversation
Dart_CObject_kNativePointerDart_CObject_kNativePointer
PR Health
Breaking changes
|
| Package | Change | Current Version | New Version | Needed Version | Looking good? |
|---|---|---|---|---|---|
| objective_c | Breaking | 9.4.1 | 9.4.1 | 10.0.0 Got "9.4.1" expected >= "10.0.0" (breaking changes) |
This check can be disabled by tagging the PR with skip-breaking-check.
API leaks ✔️
The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
| Package | Leaked API symbol | Leaking sources |
|---|
This check can be disabled by tagging the PR with skip-leaking-check.
Changelog Entry ❗
| Package | Changed Files |
|---|---|
| package:objective_c | pkgs/objective_c/lib/objective_c.dart pkgs/objective_c/lib/src/c_bindings_generated.dart pkgs/objective_c/lib/src/internal.dart pkgs/objective_c/lib/src/objective_c_bindings_generated.dart |
Changes to files need to be accounted for in their respective changelogs.
This check can be disabled by tagging the PR with skip-changelog-check.
|
Thanks for the PR @mertalev, but I think I should do this one myself. I've already got a WIP PR for this, and I'm trying to solve the problem not just for listener blocks, but also blocking blocks, and protocol implementations. |
That's definitely fair! This PR is more limited in scope and you have much more context on the issue than I do. For now, this branch is useful to address the listener block crash for my app, so I'm fine with closing this PR and using it until your changes land. If it's useful, you can use as much or as little of this branch as you like. |
Description
As per #3265, ports are safer than callbacks as they do not cause a crash when native code tries to reach an isolate that no longer exists. These crashes can occur during hot restart, when background tasks exit, and so on. This PR implements the ObjC side of the proposed changes with the same design as the jnigen fix (#3452). It delivers through a Dart port using
Dart_CObject_kNativePointer, so every outcome has an owner. Specifically:In addition to dedicated unit tests, the changes were tested locally via cupertino_http in an app that previously crashed on hot restart due to dart-lang/http#1894.
Related Issues
Part of #3265
PR Checklist
dart tool/ci.dart --alllocally and resolved all issues identified. This ensures the PR is formatted, has no lint errors, and ran all code generators. This applies to the packages part of the toplevelpubspec.yamlworkspace.CHANGELOG.mdfor the relevant packages. (Not needed for small changes such as doc typos).