fix(di): scope config-derived resources to a configuration generation (A02) - #99
Merged
Merged
Conversation
evg4b
force-pushed
the
fix/a02-generation-scoped-resources
branch
from
August 30, 2026 16:21
e2083d9 to
525289a
Compare
evg4b
changed the base branch from
fix/a01-request-tracker-non-blocking
to
main
August 30, 2026 16:21
… (A02) The container held one append-only closer slice that was drained only at process exit, so every config reload leaked a HAR writer plus its goroutine and left stale writers racing to rewrite the same archive. The cache went the other way: `factory1` memoised the first instance and silently ignored the argument, so `cache-config` changes never applied. - New `di.Runtime` owns everything derived from one `UncorsConfig` (routers, targets, HAR writers, the response cache) and releases it on `Close`. - `Uncors` builds the next generation before restarting, so a config that fails to build leaves the running proxy untouched, and closes the previous one only once the new one is live. - `Shutdown`/`Close` release the active generation, which flushes HAR writers. - `cache-config` is now genuinely hot-reloadable; the vestigial `factory1` is gone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
evg4b
force-pushed
the
fix/a02-generation-scoped-resources
branch
from
August 30, 2026 16:29
525289a to
60c67df
Compare
evg4b
marked this pull request as ready for review
August 30, 2026 16:31
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.
Fixes review finding A02 — The DI container leaks a HAR writer per reload and permanently ignores cache-config changes.
The container held one append-only closer slice that was drained only at
process exit, so every config reload leaked a HAR writer plus its goroutine and
left stale writers racing to rewrite the same archive. The cache went the other
way:
factory1memoised the first instance and silently ignored the argument,so
cache-configchanges never applied.di.Runtimeowns everything derived from oneUncorsConfig(routers,targets, HAR writers, the response cache) and releases it on
Close.Uncorsbuilds the next generation before restarting, so a config that failsto build leaves the running proxy untouched, and closes the previous one only
once the new one is live.
Shutdown/Closerelease the active generation, which flushes HAR writers.cache-configis now genuinely hot-reloadable; the vestigialfactory1isgone.
Review
One commit,
e2083d9. Step 2 of 33 in the review stack.This PR targets
fix/a01-request-tracker-non-blocking, the branch for the preceding finding, so that the diff shown here is exactly this one commit and nothing else. GitHub retargets it tomainautomatically once that base merges.make check(gofmt, gofumpt, golangci-lint, unit tests with-race, dead-code analysis, build) and thetagged integration suite pass at this commit.