Skip to content

[ffigen] Migrate callbacks to Dart_CObject_kNativePointer#3458

Closed
mertalev wants to merge 7 commits into
dart-lang:mainfrom
mertalev:fix/safe-objc
Closed

[ffigen] Migrate callbacks to Dart_CObject_kNativePointer#3458
mertalev wants to merge 7 commits into
dart-lang:mainfrom
mertalev:fix/safe-objc

Conversation

@mertalev

@mertalev mertalev commented Jul 7, 2026

Copy link
Copy Markdown

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:

  1. If the post is delivered, the per-isolate invoker handles the post, frees the invocation and releases the block
  2. If the post is refused, ownership stays with the poster, which cleans up inline
  3. If the post is sent but not handled, the VM runs the message's cleanup finalizer

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

  • I’ve reviewed the contributor guide and applied the relevant portions to this PR.
  • I've run dart tool/ci.dart --all locally 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 toplevel pubspec.yaml workspace.
  • All existing and new tests are passing. I added new tests to check the change I am making.
  • The PR is actually solving the issue. PRs that don't solve the issue will be closed. Please be respectful of the maintainers' time. If it's not clear what the issue is, feel free to ask questions on the GitHub issue before submitting a PR.
  • I have updated CHANGELOG.md for the relevant packages. (Not needed for small changes such as doc typos).
  • I have updated the pubspec package version if necessary.

@mertalev mertalev changed the title [ffigen, objective_c] Migrate callbacks to Dart_CObject_kNativePointer [ffigen] Migrate callbacks to Dart_CObject_kNativePointer Jul 7, 2026
@liamappelbe liamappelbe self-requested a review July 8, 2026 00:54
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown

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.

@liamappelbe

Copy link
Copy Markdown
Contributor

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.

@mertalev

mertalev commented Jul 8, 2026

Copy link
Copy Markdown
Author

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.

@mertalev mertalev closed this Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants