feat(engine): defer RouterContext.attach to run after registry bootstrap - #2422
Merged
Conversation
RouterContext.attach(RouterConfig) was called as the first statement of EngineWorker.doInit(), before this worker's own EngineRegistry exists and before EngineManager has processed any namespace, yet its contract already lets an implementation attach a synthesized composite namespace via RouteableContext.attachComposite() as part of that same call — a namespace registration that reads current per-worker registry state. Every existing RouterContext implementation already builds its stream factory once, at construction time in Router.supply(), and attach() just returns that value unchanged; RouterConfig itself is unused. So the stream factory never depended on attach() running early — only bindings' own construction-time capture of it did. Split the two: add RouterContext.streamFactory(), sourced by EngineWorker right after RouterContext is constructed, and change attach() to void, called once per worker only after Engine.start() has run the engine's own manager.start() bootstrap to completion, dispatched onto that worker's own thread the same way ordinary namespace attachment already is.
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
2 participants
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
RouterContext.attach(RouterConfig)ran as the first statement ofEngineWorker.doInit()— before this worker's ownEngineRegistryis constructed, and beforeEngineManagerhas processed any namespace. Its contract already lets an implementation attach a synthesized composite namespace viaRouteableContext.attachComposite()as part of that same call (RouteableContext's own javadoc: "Attaches a synthesizedNamespaceConfigto the engine alongside operator-authored namespaces"), which reads and mutates per-worker registry state that doesn't exist yet at that point in engine bootstrap.Every existing
RouterContextimplementation (EngineRouterContext, the test router, and a real composite-synthesizing router built against this SPI) already builds its stream factory once, at construction time inRouter#supply(RouteableContext), andattach()just returns that value unchanged —RouterConfigitself goes unused in every implementation. So the stream factory returned byattach()never actually depended onattach()running early duringdoInit(); only bindings' own construction-time capture ofEngineContext#streamFactory()did (every binding factory doesthis.streamFactory = context.streamFactory();in its own constructor, called fromdoInit()).This splits the two concerns:
RouterContext#streamFactory()(new) — returns the already-constructed stream factory.EngineWorkersources this immediately afterRouter#supply(RouteableContext)returns, in its own constructor — independent of whenattach()runs.RouterContext#attach(RouterConfig)— nowvoid.EngineWorker.doInit()no longer calls it. Instead,Engine#start()calls it once per worker, dispatched onto that worker's own thread (mirroring the existingEngineWorker#attach(NamespaceConfig)pattern), only afterEngineManager#start()has run the engine's bootstrap config through to completion — so any registry-dependent setup a router'sattach()performs (like attaching a composite namespace) now runs with the registry andEngineManager's live configuration already in place.No behavior change for any existing router:
EngineRouterContext#attach()becomes an empty no-op, matching what it always effectively did.Test plan
./mvnw checkstyle:check -pl runtime/engine— 0 violations./mvnw clean verify -pl runtime/engine— 228 unit tests + 210 integration tests, 0 failures/errorsGenerated by Claude Code