What is wrong
BgpServer is registered with a side-effect-ful AddHostedService factory that writes a captured local variable, and ISessionManager then reads that variable from a separate factory:
// BGPLite/Program.cs:156-176
BgpServer? bgpServer = null;
builder.Services.AddHostedService(sp =>
{
var store = sp.GetRequiredService<PeerStore>();
var prefixService = sp.GetRequiredService<IPrefixService>();
bgpServer = new BgpServer( // ← side effect on a captured local
sp.GetRequiredService<AppConfig>(),
...
(ip, asn) => store.UpsertPeer(ip, asn),
store, prefixService,
communityResolver: sp.GetRequiredService<ICommunityResolver>());
return bgpServer;
});
builder.Services.AddSingleton<ISessionManager>(sp => bgpServer!); // ← reads the captured local
This wiring is order-dependent and fragile:
- The
AddHostedService factory only runs when the hosted service starts (lazily, on StartAsync).
- The
ISessionManager factory runs when any consumer resolves ISessionManager — which can happen earlier (e.g. PrefixAutoRefreshService and PrefixSourceService both inject ISessionManager and may resolve it during their own construction / first use).
- If
ISessionManager resolves before the hosted-service factory has run, bgpServer is still null and bgpServer! throws NullReferenceException at runtime — hidden behind the null-forgiving !.
In the current configuration it happens to work because the hosted service starts before the consumers fire their first ISessionManager call, but this is incidental. Any change to registration order, an eager-singleton consumer, or a startup hook that resolves ISessionManager early will turn this into an NRE.
It is also inconsistent with the pattern already used in the same file for ManagementApi (Program.cs:182-183):
builder.Services.AddSingleton<ManagementApi>();
builder.Services.AddHostedService(sp => sp.GetRequiredService<ManagementApi>());
— here the singleton is the source of truth and the hosted service resolves it. The same pattern should be used for BgpServer.
Fix
Register BgpServer as a singleton and have both AddHostedService and ISessionManager resolve it from the container — no captured local:
builder.Services.AddSingleton(sp =>
{
var store = sp.GetRequiredService<PeerStore>();
var prefixService = sp.GetRequiredService<IPrefixService>();
return new BgpServer(
sp.GetRequiredService<AppConfig>(),
...
(ip, asn) => store.UpsertPeer(ip, asn),
store, prefixService,
communityResolver: sp.GetRequiredService<ICommunityResolver>());
});
builder.Services.AddHostedService(sp => sp.GetRequiredService<BgpServer>());
builder.Services.AddSingleton<ISessionManager>(sp => sp.GetRequiredService<BgpServer>());
Now there is exactly one BgpServer instance, owned by the DI container, and every consumer resolves the same instance regardless of order.
Acceptance
- No captured-local / mutable-field indirection between
AddHostedService and ISessionManager.
ISessionManager resolves to a non-null instance regardless of resolution order.
- Startup/shutdown ordering unchanged (hosted-service lifecycle preserved).
dotnet test startup tests green.
Refs: #93 / #88 / #98 (architecture-review cluster — this is the DI-wiring instance of the same structural debt).
What is wrong
BgpServeris registered with a side-effect-fulAddHostedServicefactory that writes a captured local variable, andISessionManagerthen reads that variable from a separate factory:This wiring is order-dependent and fragile:
AddHostedServicefactory only runs when the hosted service starts (lazily, onStartAsync).ISessionManagerfactory runs when any consumer resolvesISessionManager— which can happen earlier (e.g.PrefixAutoRefreshServiceandPrefixSourceServiceboth injectISessionManagerand may resolve it during their own construction / first use).ISessionManagerresolves before the hosted-service factory has run,bgpServeris stillnullandbgpServer!throwsNullReferenceExceptionat runtime — hidden behind the null-forgiving!.In the current configuration it happens to work because the hosted service starts before the consumers fire their first
ISessionManagercall, but this is incidental. Any change to registration order, an eager-singleton consumer, or a startup hook that resolvesISessionManagerearly will turn this into an NRE.It is also inconsistent with the pattern already used in the same file for
ManagementApi(Program.cs:182-183):— here the singleton is the source of truth and the hosted service resolves it. The same pattern should be used for
BgpServer.Fix
Register
BgpServeras a singleton and have bothAddHostedServiceandISessionManagerresolve it from the container — no captured local:Now there is exactly one
BgpServerinstance, owned by the DI container, and every consumer resolves the same instance regardless of order.Acceptance
AddHostedServiceandISessionManager.ISessionManagerresolves to a non-null instance regardless of resolution order.dotnet teststartup tests green.Refs: #93 / #88 / #98 (architecture-review cluster — this is the DI-wiring instance of the same structural debt).