Skip to content

[Windows] Add public API to post task to platform thread#187365

Merged
auto-submit[bot] merged 6 commits into
flutter:masterfrom
loic-sharma:sync_messages
Jun 23, 2026
Merged

[Windows] Add public API to post task to platform thread#187365
auto-submit[bot] merged 6 commits into
flutter:masterfrom
loic-sharma:sync_messages

Conversation

@loic-sharma

Copy link
Copy Markdown
Member

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-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.

@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 1, 2026
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically a: desktop Running on desktop team-windows Owned by the Windows platform team labels Jun 1, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 1, 2026
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 1, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 1, 2026
@loic-sharma loic-sharma added the CICD Run CI/CD label Jun 1, 2026
@loic-sharma loic-sharma marked this pull request as ready for review June 1, 2026 05:05
@loic-sharma loic-sharma requested a review from a team as a code owner June 1, 2026 05:05
@loic-sharma loic-sharma requested review from mattkae and removed request for a team June 1, 2026 05:06

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread engine/src/flutter/shell/platform/windows/public/flutter_windows.h Outdated
Comment thread engine/src/flutter/shell/platform/windows/flutter_windows.cc Outdated
Comment thread engine/src/flutter/shell/platform/windows/client_wrapper/flutter_engine.cc Outdated
Comment thread engine/src/flutter/shell/platform/windows/flutter_windows_unittests.cc Outdated
Comment thread engine/src/flutter/shell/platform/windows/flutter_windows_unittests.cc Outdated

@mattkae mattkae left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would agree with gemini on the deleter if we deem it necessary

Comment thread engine/src/flutter/shell/platform/windows/client_wrapper/flutter_engine.cc Outdated
@knopp

knopp commented Jun 1, 2026

Copy link
Copy Markdown
Member

I'm wondering if we shouldn't go with PostOnPlatformThread and IsCurrentThreadPlatformThread, or something similar. I get that combining this may be useful sometimes, but the different behavior - call performed immediately or scheduled - depending on current thread, bothers me a bit.

@loic-sharma loic-sharma marked this pull request as draft June 15, 2026 20:40
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 19, 2026
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 19, 2026
@loic-sharma loic-sharma marked this pull request as ready for review June 19, 2026 19:52
@loic-sharma loic-sharma requested a review from mattkae June 19, 2026 19:52
@loic-sharma

Copy link
Copy Markdown
Member Author

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 :)

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread engine/src/flutter/shell/platform/windows/flutter_windows.cc
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 19, 2026
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Jun 19, 2026
Comment thread engine/src/flutter/shell/platform/windows/public/flutter_windows.h
@github-actions github-actions Bot removed the CICD Run CI/CD label Jun 19, 2026
@loic-sharma loic-sharma added the CICD Run CI/CD label Jun 19, 2026
@loic-sharma loic-sharma requested a review from knopp June 19, 2026 22:46

@mattkae mattkae left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good by me!

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 23, 2026
@auto-submit auto-submit Bot added this pull request to the merge queue Jun 23, 2026
Merged via the queue into flutter:master with commit c8e3959 Jun 23, 2026
23 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 23, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Jun 23, 2026
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
via-guy pushed a commit to via-guy/flutter that referenced this pull request Jun 26, 2026
)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: desktop Running on desktop CICD Run CI/CD engine flutter/engine related. See also e: labels. platform-windows Building on or for Windows specifically team-windows Owned by the Windows platform team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants