Skip to content

Fix stopped lifecycle callback ordering - #6442

Open
jbdevprimary wants to merge 1 commit into
fyne-io:developfrom
jbcom:fix/lifecycle-stopped-ordering
Open

Fix stopped lifecycle callback ordering#6442
jbdevprimary wants to merge 1 commit into
fyne-io:developfrom
jbcom:fix/lifecycle-stopped-ordering

Conversation

@jbdevprimary

@jbdevprimary jbdevprimary commented Jul 26, 2026

Copy link
Copy Markdown

Description:

Run the GLFW stopped lifecycle callback synchronously on the app/main goroutine before the function queue is drained.

The shutdown loop currently queues the composite stopped callback after Terminate and immediately drains the function queue. Depending on timing, the callback can be drained without execution and then run through the post-drain direct path from another goroutine. That breaks lifecycle ordering, moves the callback off the app goroutine, and can race cleanup.

This keeps the existing Terminate-before-OnStopped ordering, but invokes the already-composed callback directly from runGL before queue drain. The user hook still runs before the internal preferences hook. Mobile behavior is unchanged.

A fresh-subprocess regression proves both stopped hooks run exactly once, on the main goroutine, before drained is set; Driver.Run returns; and a background DoAndWait queued during shutdown is released without its canceled callback executing.

Related to #5748, but deliberately does not claim to fix the broader post-shutdown Do/DoAndWait queue race.

Local focused, race, ci/migrated_fynedo, no_glfw, WebAssembly build, lint, vet, and formatting checks pass. The full platform/GLFW matrix is left to upstream CI because the local macOS suite has an unrelated nil-monitor failure and local Linux/ARM64 golden images differ.

Checklist:

  • Tests included.
  • Lint and formatter run with no errors.
  • Tests all pass across the full platform matrix; upstream CI pending.

return a.driver
}

func TestRunLifecycleShutdown(t *testing.T) {

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.

This test is so complicated to read I don't even really understand what it's doing. I especially don't think a unit test should ever need an exec.Command call, just let the go test runner invoke the test. The actual code change is so small I'd be fine with reverting both test files and just shipping the one line change. The l.QueueEvent is mostly a remnant of when event callbacks ran in a separate thread pre-Fyne 2.6 anyway (you can see in the same file a bit above your change that we just directly call f() for the OnStarted callback already).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants