Fallout-build / Fallout-build/Fallout

[Foundation] Cover the cross-invocation subscription leak BuildContext fixes

Open
#546 0 comments 0 reactions 0 assignees View on GitHub
enhancement target/vCurrent
Dominant language
C#
Stars
154
Forks
19
Avg merge
1d 22h
Merged PRs (30d)
15

Description

Follow-up from [#451](https://github.com/Fallout-build/Fallout/pull/451) (FT-2, [#307](https://github.com/Fallout-build/Fallout/issues/307)).

## Problem

FT-2 claims a behavioural fix that nothing asserts: because the handler list is owned by `BuildContext` and discarded on dispose, `BuildExecutor`'s `ExecuteAssuredTargets` subscription — never explicitly unsubscribed — no longer survives across in-process invocations.

That's the one externally-visible improvement in the PR, and it has no test. A regression would be silent, and it only shows up in the multi-`Execute`-per-process scenarios (specs, hosted runners) that are hardest to notice breaking.

Related: `BuildContext.Dispose` unsubscribing `Console.CancelKeyPress` and `ToolOptions.Created` is also unasserted. `ToolOptions.Created` is reachable from a spec (it fires from `ToolOptions`' constructor), though the observable is `VerbosityMapping.Apply` setting a mapped property; `Console.CancelKeyPress` isn't raisable at all. Fold in if the seam from #545 makes it cheap.

## Outcome

Running two builds in one process demonstrably starts the second with no handlers carried over from the first.

## Acceptance criteria

- [ ] A spec runs `BuildManager.Execute` twice in-process and asserts the second run starts with no handlers left from the first.
- [ ] The spec fails if the per-run handler list is made static again.
- [ ] Covered without loosening `internal` accessibility beyond the test assemblies.

Contributor guide

Open the contributing guide

Research direction

Start with the existing specs and the BuildManager.Execute entry point, then trace BuildContext disposal and BuildExecutor's ExecuteAssuredTargets subscription. Add an in-process spec that executes twice and verifies the second run starts without handlers from the first. The spec should also fail if the per-run handler list becomes static again, without loosening internal accessibility beyond the test assemblies.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
build-system, testing-qa
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.