Skip to content

gui: reclaim layout callback lifetimes#2

Open
GGRei wants to merge 1 commit into
mainfrom
fix/gui58-closure-lifetime-reclaim
Open

gui: reclaim layout callback lifetimes#2
GGRei wants to merge 1 commit into
mainfrom
fix/gui58-closure-lifetime-reclaim

Conversation

@GGRei

@GGRei GGRei commented Jun 18, 2026

Copy link
Copy Markdown
Owner

Summary

This uses the closure lifetime/reclaim API from vlang/v#27483 to reclaim transient layout callbacks in v-gui.

Window.update() now builds each layout tree inside a layout callback lifetime frame, keeps the current layout alive, and reclaims old frame-owned closure contexts after the new layout has been installed.

Persistent callbacks created during layout, such as animations or hover-created callbacks, are kept outside the transient reclaim scope.

Changes

  • Add a small v-gui wrapper around the V closure lifetime API.
  • Track layout callback closures during Window.update().
  • Reclaim old layout-owned callbacks after replacing the layout tree.
  • Dispose layout callback lifetime state during window cleanup.
  • Keep persistent animations created during layout outside frame reclaim.
  • Suspend layout callback tracking around hover callback dispatch.
  • Add regression coverage for bounded layout callback memory growth, current-layout callback survival, persistent animations, hover-created callbacks, mouse lock callbacks, and idempotent cleanup.

Dependency

This PR depends on vlang/v#27483.

It should not be merged until vlang/v#27483 is merged and v-gui CI uses a V compiler that includes the closure lifetime/reclaim API.

Tests

/tmp/vgui_lifetime_v_3d974672d fmt -verify \
  window.v \
  window_update.v \
  a11y_tree.v \
  closure_lifetime.v \
  _window_lifetime_test.v \
  layout.v \
  view_svg.v \
  view_progress_bar.v \
  view_sidebar.v \
  animation.v

git diff --check

VJOBS=1 /tmp/vgui_lifetime_v_3d974672d -no-parallel test _window_lifetime_test.v
VJOBS=1 /tmp/vgui_lifetime_v_3d974672d -no-parallel -gc boehm test _window_lifetime_test.v
VJOBS=1 /tmp/vgui_lifetime_v_3d974672d -no-parallel -gc none test _window_lifetime_test.v

VJOBS=1 /tmp/vgui_lifetime_v_3d974672d -no-parallel -cc clang -gc boehm test \
  _gc_leak_test.v \
  _window_refresh_test.v \
  _event_test.v \
  _window_queue_test.v

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c2b94d219a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread animation.v Outdated
@GGRei GGRei force-pushed the fix/gui58-closure-lifetime-reclaim branch from c2b94d2 to 593dbfd Compare June 18, 2026 12:37
@GGRei

GGRei commented Jun 18, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 593dbfdba7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread window_update.v Outdated
Comment thread window_update.v Outdated
Comment thread view_sidebar.v Outdated
@GGRei GGRei force-pushed the fix/gui58-closure-lifetime-reclaim branch from 593dbfd to 0478d8a Compare June 18, 2026 13:17
@GGRei

GGRei commented Jun 18, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0478d8a7dd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread window_update.v Outdated
Comment thread window_update.v Outdated
Comment thread window_update.v Outdated
@GGRei GGRei force-pushed the fix/gui58-closure-lifetime-reclaim branch from 0478d8a to 7fabf3f Compare June 18, 2026 15:24
@GGRei

GGRei commented Jun 18, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7fabf3ffcf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread window_update.v
window.layout = window.compose_layout(mut view)
view_clear(mut view)
}) or { panic(err) }
window.reclaim_old_layout_callbacks()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Pin async CRUD callbacks before reclaiming layout closures

With this reclaim now running after every layout pass, a data grid CRUD save can outlive the one retained layout frame: view_data_grid_crud.v:589-594 starts a background mutation from the Save click and captures on_crud_error, on_rows_change, and on_selection_change into the queued completion without any pin/suspend. Since event_fn requests a layout refresh after the click, any save that takes more than a frame can later call callbacks that were allocated in the old layout lifetime and have already been reclaimed, leading to use-after-free or dropped/corrupt completion handling.

Useful? React with 👍 / 👎.

Comment thread drag_reorder.v
parent_id: state.parent_id
})
if drag_reorder_dispatch_drop(drag_key, mut w) {
w.animate_layout(LayoutTransitionCfg{})

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Capture reorder snapshots before dispatching drop

In the drop path, drag_reorder_dispatch_drop invokes the widget handler and drag_reorder_apply_drop, which calls the user's on_reorder, before this snapshot is captured. If an on_reorder callback synchronously rebuilds or replaces the view (for example by calling w.update() or w.update_view()), animate_layout() captures the already-updated layout, so the final drop loses its FLIP transition. The previous code captured before on_reorder, which matches the ordering required by animation_layout.v.

Useful? React with 👍 / 👎.

Comment thread data_source_grid.v
Comment on lines +293 to +294
window.suspend_layout_callback_tracking(fn [source, req, grid_id, next_request_id, caps, mut window] () {
spawn fn [source, req, grid_id, next_request_id, caps] (mut w Window) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Pin inline data-source callbacks until fetch completion

This suspend only detaches the newly-created spawn wrapper; it does not keep closures that are already stored inside source alive. If a view constructs a GridOrmDataSource or custom DataGridDataSource inline with a fetch_fn/method closure capturing layout locals, the background source.fetch_data(req) can run after the next layout reclaim and invoke a reclaimed closure. The same pattern exists in the list-box source path, so these async source requests need a reclaim pin until the fetch queues its result, or the source callbacks need to be copied outside the layout lifetime before this point.

Useful? React with 👍 / 👎.

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.

1 participant