Repository navigation
Fix use-after-free of FlTaskRunner timeout on shutdown - #194525
Conversation
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
There was a problem hiding this comment.
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.
|
This change also fixes #190620, and a second effect of the same Measured on master d016ca4, profile mode, GTK on Wayland (glib 2.90.1), and macOS on the same commit:
With an In a standalone GLib program on the same machine (500 runs each), a due task is dispatched 1072 µs late with 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 |
818def7 to
c1e77c3
Compare
|
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. |
|
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. |
|
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. |
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
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
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