fix: dispose factory outputs hidden behind a non-disposable return type#33
Conversation
A factory registration computes disposability from the producer method's declared return type, so a factory declared `static IFoo Create()` that builds a `DisposableFooImpl : IFoo, IDisposable` was flagged non-disposable and never tracked, leaking the instance when the owning scope or root tore down. Constructed types use the concrete `info.Symbol` (always truthful) and a pre-built `Instance` is intentionally not owned, so the lie is possible only for factory production. For a factory whose declared return type is not itself `IDisposable` yet could produce one at runtime (an interface, a type parameter, or a non-sealed class), the emitted resolver now tracks the realized instance behind a runtime `is IDisposable` check instead of trusting the static flag. This retains only genuinely-disposable outputs (no retention cost otherwise) and closes the gap across the synchronous fresh resolver, the async fresh/caching registration, and the caching singleton/scoped disposal add. A sealed return type that is not `IDisposable` cannot hide one, so it emits no check (and a runtime `is IDisposable` against it would not even compile). The existing `__gate` lock and `__disposed` re-check semantics are preserved for factory outputs, so one built during a concurrent dispose is still disposed rather than leaked. The static `InstanceModel.IsDisposable` semantics are unchanged: the AWT118 root-accumulation analyzer and strict-lifetime withholding keep their compile-time behavior. AWT118 still under-fires for factory-hidden disposables (the same declared-type gap, but compile-time) - that is left to a separate body-analysis linter PR. `IAsyncDisposable` remains out of scope, matching the current `IDisposable`-only drain.
🚀 Benchmark ResultsDetails
Details
Details
|
…ly-once disposal Add a runtime test for a factory whose declared return type extends IAsyncInitializable but hides a concrete IDisposable behind it, exercising the async creator path (EmitAsyncDisposableRegistration with the runtime is-IDisposable check) that the synchronous hidden-disposable tests do not reach. It asserts the instance is both initialized and disposed exactly once. Convert the HiddenDisposable and PlainDisposable test fixtures from a bool Disposed toggle to an instance-level DisposeCount, and tighten the singleton/scoped/transient/no-regression assertions to expect exactly one disposal. A double-add to the disposal list would now fail rather than pass silently.
The CachingResolver constructor reached 8 parameters after the runtime disposal check was added (modifiers, type, method, field, construction, disposable, runtimeCheck, comment), tripping the maintainability limit. The two disposal booleans carry an invariant - a runtime check is only ever set when the static disposable flag is not - so they collapse cleanly into a single DisposalTracking value (None, Static, Runtime), bringing CachingResolver to 7 parameters and FreshResolver to 6. This also removes the repeated 'IsDisposable || TracksDisposalAtRuntime' computation at the five resolver call sites and the parallel pair of bool checks in the emitter bodies. The generated output is unchanged (the source-generator snapshot tests pass verbatim).
31e39af to
a3e67df
Compare
… as a tuple switch Convert the eleven member-level line comments in Emitter (IsWithheld, IsFuncWithheld, the DisposalTracking enum, DisposalOf, EmitsSync, the four guidance-message helpers, Withheld, and OwnedBare) to /// XML-doc summaries, matching the file's existing convention. Generic types that appear in the prose are wrapped in <c> with their angle brackets escaped (Owned<T>, Func<…, Owned<T>>) so the comments remain well-formed XML; in-body comments are left as line comments since they document statements rather than members. Rewrite DisposalOf's body as a tuple switch over (RuntimeDisposalCheck, IsDisposable) instead of nested ternaries. The mapping is unchanged - (true, _) => Runtime, (_, true) => Static, (_, _) => None - so the generated output is identical and the source-generator snapshot tests pass verbatim.
|
…ind a non-disposable return type (#33) by Valentin Breuß
…ind a non-disposable return type (#33) by Valentin Breuß



A factory registration computes disposability from the producer method's declared return type, so a factory declared
static IFoo Create()that builds aDisposableFooImpl : IFoo, IDisposablewas flagged non-disposable and never tracked, leaking the instance when the owning scope or root tore down. Constructed types use the concreteinfo.Symbol(always truthful) and a pre-builtInstanceis intentionally not owned, so the lie is possible only for factory production.For a factory whose declared return type is not itself
IDisposableyet could produce one at runtime (an interface, a type parameter, or a non-sealed class), the emitted resolver now tracks the realized instance behind a runtimeis IDisposablecheck instead of trusting the static flag. This retains only genuinely-disposable outputs (no retention cost otherwise) and closes the gap across the synchronous fresh resolver, the async fresh/caching registration, and the caching singleton/scoped disposal add. A sealed return type that is notIDisposablecannot hide one, so it emits no check (and a runtimeis IDisposableagainst it would not even compile). The existing__gatelock and__disposedre-check semantics are preserved for factory outputs, so one built during a concurrent dispose is still disposed rather than leaked.The static
InstanceModel.IsDisposablesemantics are unchanged: the AWT118 root-accumulation analyzer and strict-lifetime withholding keep their compile-time behavior. AWT118 still under-fires for factory-hidden disposables (the same declared-type gap, but compile-time) - that is left to a separate body-analysis linter PR.IAsyncDisposableremains out of scope, matching the currentIDisposable-only drain.