Repository navigation
Fix retained lifetimes and expand collection coverage - #3446
Merged
Merged
Conversation
Unchanged master: observer fixture has 2 collection failures and 2 passing controls; WPF lifetime fixtures have 5 failures and 4 passing controls. Covers abandoned tracker, disposed-manager collection, loader replacement, disposal order, late callbacks and nested observer recreation. # Conflicts: # tests/Avalonia/Prism.Avalonia.Tests/Prism.Avalonia.Tests.csproj # tests/Wpf/Prism.Wpf.Tests/Prism.Wpf.Tests.csproj
Clear detached observer sources and ignore callbacks already captured before detachment. Untrack delayed region behaviors even when the weak host has died, using a shared tracker lock. Detach module-loader subscriptions before disposing loaders, make disposal idempotent, ignore late callbacks, and reject new work after disposal. Validation: Core net10 342 passed; WPF net472 targeted lifecycle suites 43 passed; Avalonia net10 40 passed with 3 pre-existing skips. All 23 new lifetime cases passed across those hosts. Uno managed net10 library builds. Baseline failing regressions are preserved in the preceding test commit. No device or native Uno UI qualification.
Carry over weak/strong subscription, queued delivery, composite/async command, journal payload and static factory controls. Keep existing observer regressions once, exercise MAUI scoped teardown through a retained mock root, and serialize dispatcher-sensitive tests with the existing lifecycle collection. Retain current-master DefaultView coverage without duplicating it. Validation: Core net10 355 passed; MAUI managed net10 356 passed; WPF net472 491 passed; Avalonia net10 435 passed and 41 existing skips; Uno desktop/headless suite 451 passed with existing duplicate API-inventory IDs. No new skips or KnownRetention exclusions. Native/device/real-container paths remain unqualified. Production fix scope is unchanged.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e83e379a30
ℹ️ 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".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of Change
Clearing an observed child left the former child rooted by its observer node; abandoning a nonvisual region host left its behavior in a static tracker; and a retained module type loader kept a disposed ModuleManager alive through event subscriptions. Release each root and preserve re-created observer/region behavior.
Expand deterministic lifetime coverage for weak/strong event subscriptions, queued callbacks, composite and async commands, journal payloads, factory replacement, region recreation, module disposal and MAUI deferred teardown/ancestor detachment. Tests use non-inlined lifetime helpers, weak references, explicit retained roots and controlled GC/queues. Existing observer cases are not duplicated; merged #3442 DefaultView tests remain included.
Bugs Fixed
Related: #3442 (merged lifecycle coverage). The three fixes were reproduced on unchanged production code before implementation; the regression commit is preserved in this branch. Historical #803 and #844 describe different ownership paths and are not claimed as fixed here.
API Changes
None: no public signatures added or changed.
Behavioral Changes
ModuleManager.Run and LoadModule now throw ObjectDisposedException after disposal. Repeated Dispose calls do not dispose loaders again, and callbacks received after disposal do not initialize modules or report progress. Disposal waits for in-flight loader callbacks. When called reentrantly from a callback, it detaches subscriptions immediately and defers loader cleanup until the outer callback exits, allowing loaders to wait for their worker callbacks without holding the callback lock.
Validation
All new lifetime tests ran as ordinary assertions: no deliberate-failure traits, new skips or failure filters. Builds/tests ran serially using existing SDKs/feeds; Avalonia used its installed build-task DLL via a command-local property.
MAUI root/scope and dispatcher coverage uses mocks and controlled queues. This does not qualify real containers, rendered native windows, native Windows cold activation, Android/iOS/Mac/device lifecycles, or every Prism API. No audit reports, scratch patches, scripts or standalone harness projects are included. Hosted CI will validate this PR's exact head separately.
Review follow-up (bf36a4d): reproduced and fixed reentrant observer detachment and in-flight/reentrant module disposal; isolated the static factory and region fixtures and reset ContainerLocator during teardown. Local validation: Core 356 passed, WPF 494 passed (all seven module cases additionally revalidated after converting the race test to async waits), Avalonia 438 passed / 41 existing skips. The three introduced CS8618 warnings were removed with accurate nullable fixture state in 9a87cab. Uno discovery excludes 28 existing API-inventory overload rows, not lifecycle cases; the TRX verifies all 19 DefaultView lifecycle cases passed. This branch adds no new Uno lifetime-fixture includes.
PR Checklist