[Windows] Add public API to post task to platform thread#187365
Conversation
11750cf to
7b8e36c
Compare
7b8e36c to
510dd43
Compare
510dd43 to
75355db
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces RunNowOrPostPlatformThreadTask to schedule callbacks on the platform thread from any thread, along with corresponding unit tests. The feedback suggests adding a deleter callback to the C API to prevent memory leaks if tasks are discarded before execution, which requires updating the engine wrapper, stubs, and tests to support this parameter.
mattkae
left a comment
There was a problem hiding this comment.
I would agree with gemini on the deleter if we deem it necessary
|
I'm wondering if we shouldn't go with |
75355db to
6fcb4ca
Compare
6fcb4ca to
2bcc000
Compare
|
Thanks for the feedback! I switched to always posting a task (instead of running immediately if on the platform thread) and addressed the leak if the engine is shutdown while one of these tasks is pending. This should be ready for another review :) |
There was a problem hiding this comment.
Code Review
This pull request introduces PostPlatformThreadTask to the Windows platform shell, enabling callbacks to be scheduled on the platform thread from any thread, with support for cancellation during engine shutdown. Unit tests are also added to verify this behavior. The review feedback highlights critical safety issues where missing validation for empty or null callbacks could lead to undefined behavior, exceptions across the C ABI boundary, or null pointer dereferences. Code suggestions are provided to validate these callbacks and optimize the task context structure.
flutter/flutter@e228771...87224e0 2026-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 5cae7f9ada62 to 3a66ea7b9aaa (1 revision) (flutter/flutter#188379) 2026-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 1e6c246bb73a to 5cae7f9ada62 (2 revisions) (flutter/flutter#188370) 2026-06-23 engine-flutter-autoroll@skia.org Roll Skia from 766f21ae61dc to ffac3e91fbc7 (24 revisions) (flutter/flutter#188366) 2026-06-23 737941+loic-sharma@users.noreply.github.com [Windows] Add public API to post task to platform thread (flutter/flutter#187365) 2026-06-23 engine-flutter-autoroll@skia.org Roll Dart SDK from 7ab0179ce4d4 to 1e6c246bb73a (1 revision) (flutter/flutter#188354) 2026-06-23 robert.ancell@canonical.com Fix byte/character offset confusion in FlAccessibleTextField (flutter/flutter#188138) 2026-06-22 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from Lm76V7lvxVA0r1De5... to RymJjIj7dd5vQ3Cnh... (flutter/flutter#188353) 2026-06-22 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#188355) 2026-06-22 154381524+flutteractionsbot@users.noreply.github.com Sync CHANGELOG.md from stable (flutter/flutter#188331) 2026-06-22 engine-flutter-autoroll@skia.org Roll Skia from 5fbb9bbd889c to 766f21ae61dc (2 revisions) (flutter/flutter#188184) 2026-06-22 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 6.0.3 to 7.0.0 in the all-github-actions group (flutter/flutter#188350) 2026-06-22 robert.ancell@canonical.com Use g_signal_connect_object in the Linux embedder (flutter/flutter#188241) 2026-06-22 robert.ancell@canonical.com Disconnect from parent window signal when view is destroyed (flutter/flutter#185521) 2026-06-22 rmacnak@google.com Remove many absolute paths from build commands. (flutter/flutter#187765) 2026-06-22 haiderqadir.hq@gmail.com Fix spelling mistake in documentation (wether → whether) (flutter/flutter#186141) 2026-06-22 engine-flutter-autoroll@skia.org Roll Dart SDK from a748c4b15399 to 7ab0179ce4d4 (2 revisions) (flutter/flutter#188332) 2026-06-22 robert.ancell@canonical.com [Linux] Move compositor shader into its own GObject (flutter/flutter#188144) 2026-06-22 bkonyi@google.com Add agent skills for orchestrating cherry-picks to stable and beta channels (flutter/flutter#187860) 2026-06-22 engine-flutter-autoroll@skia.org Roll Packages from c516c92 to cd5194a (1 revision) (flutter/flutter#188312) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC stuartmorgan@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
) Addresses flutter#79213 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Addresses #79213
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.