Skip to content

Fix use-after-free of FlTaskRunner timeout on shutdown - #194525

Merged
auto-submit[bot] merged 4 commits into
flutter:masterfrom
robert-ancell:linux-task-runner-source
Oct 21, 2026
Merged

auto-submit[bot] merged 4 commits into
flutter:masterfrom
robert-ancell:linux-task-runner-source

Conversation

@robert-ancell

@robert-ancell robert-ancell commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

FlTaskRunner rescheduled by removing and re-adding a timeout source. If another thread posted a task while the main loop was dispatching the timeout, the new timeout became untracked and could fire after the runner was freed. Use a single source for the lifetime of the runner and reschedule it by setting its ready time, and keep the runner alive until its mutex is released in case a task drops the last reference.

Fixes #193742
Fixes #190620

FlTaskRunner rescheduled by removing and re-adding a timeout source. If
another thread posted a task while the main loop was dispatching the
timeout, the new timeout became untracked and could fire after the runner
was freed. Use a single source for the lifetime of the runner and
reschedule it by setting its ready time, and keep the runner alive until
its mutex is released in case a task drops the last reference.

Fixes flutter#193742
@robert-ancell
robert-ancell requested a review from a team as a code owner October 16, 2026 02:40
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label Oct 16, 2026
@robert-ancell
robert-ancell requested a review from mattkae October 16, 2026 02:41
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. platform-linux Building on or for Linux specifically a: desktop Running on desktop team-linux Owned by the Linux platform team labels Oct 16, 2026

@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 refactors FlTaskRunner in the Linux platform shell to use a custom GSource instead of repeatedly adding and removing timeout sources via g_timeout_add. It updates task scheduling to use g_source_set_ready_time, manages the lifecycle of the source during disposal, and splits resource cleanup between dispose and finalize. Additionally, new unit tests are added to verify that disposing the runner removes its main loop source, that tasks can safely drop the last reference to the runner, and that concurrent task posting from multiple threads works correctly. There are no review comments, so no additional feedback is provided.

@YeungKC

YeungKC commented Oct 16, 2026

Copy link
Copy Markdown
Member

This change also fixes #190620, and a second effect of the same g_timeout_add(remaining / 1000 + 1) line: a task that is already due waits at least 1 ms. On Linux the UI task runner is the platform task runner, so every Timer.run, isolate message reply, and SchedulerBinding.scheduleTask callback has this delay.

Measured on master d016ca4, profile mode, GTK on Wayland (glib 2.90.1), and macOS on the same commit:

Linux macOS
Timer.run round trip 1107 µs 20 µs
Round trip of a message to another isolate 1130 µs 28–88 µs

With an LD_PRELOAD shim that changes the 1 ms timeouts to 0 (an experiment only), the isolate round trip on Linux is 15 µs.

In a standalone GLib program on the same machine (500 runs each), a due task is dispatched 1072 µs late with g_timeout_add(ms + 1), and about 1 µs late with a GSource and g_source_set_ready_time. A task that is due at a random time within 16 ms is 577 µs late on average (p90 979 µs) with the old code, and 86 µs (p90 95 µs) with the ready time.

Could you add "Fixes #190620" to the description? A regression test could post a task with a target time of now, and then check that it runs after one g_main_context_iteration(nullptr, FALSE). With the old code the task does not run, because the 1 ms timeout is not ready. With the ready time it runs. I checked this behavior in a standalone GLib program, not in the engine test harness.

mattkae
mattkae previously approved these changes Oct 19, 2026

@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!

@robert-ancell
robert-ancell force-pushed the linux-task-runner-source branch from 818def7 to c1e77c3 Compare October 20, 2026 03:58
@robert-ancell robert-ancell added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@auto-submit

auto-submit Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/194525, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@robert-ancell robert-ancell added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@auto-submit

auto-submit Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/194525, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

mattkae
mattkae previously approved these changes Oct 20, 2026
@mattkae mattkae added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@auto-submit

auto-submit Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/194525, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label.

@robert-ancell robert-ancell added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 20, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into flutter:master with commit cb1b233 Oct 21, 2026
23 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 21, 2026
LongCatIsLooong pushed a commit to LongCatIsLooong/flutter that referenced this pull request Oct 22, 2026
FlTaskRunner rescheduled by removing and re-adding a timeout source. If
another thread posted a task while the main loop was dispatching the
timeout, the new timeout became untracked and could fire after the
runner was freed. Use a single source for the lifetime of the runner and
reschedule it by setting its ready time, and keep the runner alive until
its mutex is released in case a task drops the last reference.

Fixes flutter#193742
Fixes flutter#190620
suryaKommana2662 pushed a commit to suryaKommana2662/flutter that referenced this pull request Oct 29, 2026
FlTaskRunner rescheduled by removing and re-adding a timeout source. If
another thread posted a task while the main loop was dispatching the
timeout, the new timeout became untracked and could fire after the
runner was freed. Use a single source for the lifetime of the runner and
reschedule it by setting its ready time, and keep the runner alive until
its mutex is released in case a task drops the last reference.

Fixes flutter#193742
Fixes flutter#190620
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-linux Building on or for Linux specifically team-linux Owned by the Linux platform team

Projects

None yet

3 participants