Skip to content

fix: plug remaining per-render leaks in converted component props - #5716

Open
jkelleyrtp wants to merge 3 commits into
mainfrom
devin/1785295760-props-leak-permutations
Open

fix: plug remaining per-render leaks in converted component props#5716
jkelleyrtp wants to merge 3 commits into
mainfrom
devin/1785295760-props-leak-permutations

Conversation

@jkelleyrtp

@jkelleyrtp jkelleyrtp commented Jul 29, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #5711: a counting-allocator harness run over ~28 prop/macro permutations found three more classes of unbounded per-render growth that the child-owned-props fix didn't cover.

1. Sync-storage allocations escape the props owner. The generated props owner was a single Owner<UnsyncStorage>, and with_owner only overrides the thread-local owner for one storage type. Any SyncStorage allocation made during a prop conversion fell back to the parent scope's sync owner and leaked once per render:

  • ReadSignal<T, SyncStorage> props converted from sync mapped signals (~2.5 KB/render in the harness — the exact Memory leak #5671 bug, sync flavor)
  • #[props(default = Store::new(..))] store props (StoreSubscriptions is always CopyValue<_, SyncStorage>; ~10 KB/render)

Fix: props now hold a PropsOwner { unsync: Owner<UnsyncStorage>, sync: Owner<SyncStorage> } and conversions run under with_props_owner, which overrides both thread-local owners. Updated the props macro codegen and the hand-expanded SuspenseBoundaryProps builder.

2. point_to memoization accumulates stale subscriptions. Props like val: ReadSignal<String> set from a plain value create a fresh signal (with a fresh subscriber list) every parent render. Memoization then runs old.point_to(new), which did:

for subscriber in old.subscribers() {
    subscriber.subscribe(new_subscriber_list.clone()); // adds a new Arc to the RC's HashSet
}

Since the memoized child never reruns, its ReactiveContext never clears subscriptions, so it accumulated one dead subscriber-list Arc per render (~5 KB/render for 20 children). Fix: added ReactiveContext::unsubscribe, and ReadSignal::point_to / Signal::point_to now drop the subscription to the old list after moving subscribers to the new one (guarded by Subscribers::ptr_eq so pointing at a signal sharing the same list — e.g. mapped signals over the same source — doesn't unsubscribe the child from a live list).

3. #[props(into)] conversions the macro can't classify run in the parent owner. child_owned_type matches on the written type name, so a signal behind a type alias (e.g. type AliasSignal = ReadSignal<usize>; + #[props(into)]) was treated as a regular into-field and converted outside with_props_owner (~6 KB/render). Fix: has_child_owned_fields was broadened to has_owned_fields (signal-like, auto_into, or a non-blank default), and all into conversions and default expressions now run under the props owner. Structs with only trivial fields (children, plain Option<T> props, blank Default::default() defaults) still skip owner creation, so the common paths pay no extra allocation.

Verified: ~28 permutations flat over 2000 rerenders (mapped store/signal props across Store/ReadStore/WriteStore/ReadSignal/WriteSignal/ReadOnlySignal, sync storage, defaults, aliases, callbacks, Option handlers, spread/extends attrs, SuspenseBoundary fallbacks, pass-through chains, element listeners, use_callback). Added packages/core/tests/props_memory_leak.rs as regression coverage.

Known remaining footgun (not fixable in the macro): explicitly converting in the render body, e.g. OptChild { id: Store::from(item) } or ReadSignal::from(sig) for an Option<...>-typed prop, allocates under the parent scope and still grows per render — the conversion happens before the setter, same class as calling Signal::new in a render body. Option-wrapped signal/store props currently have no SuperFrom impls, so the macro never sees an unconverted value for them.

Link to Devin session: https://dioxus.staging.devinenterprise.com/sessions/893180b21fbb4d40874a9ec85d17f4b3
Requested by: @jkelleyrtp

@jkelleyrtp
jkelleyrtp requested a review from a team as a code owner July 29, 2026 03:30
@jkelleyrtp jkelleyrtp self-assigned this Jul 29, 2026
@staging-devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

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