Skip to content

demo(17-captive-dependency): 17 — Singleton background service captures a scoped dependency#166

Open
EricCogen wants to merge 1 commit intomainfrom
demo/17-captive-dependency
Open

demo(17-captive-dependency): 17 — Singleton background service captures a scoped dependency#166
EricCogen wants to merge 1 commit intomainfrom
demo/17-captive-dependency

Conversation

@EricCogen
Copy link
Copy Markdown
Owner

17 — Singleton background service captures a scoped dependency

Expected verdict: ❌ Fails — GauntletCI should fire GCI0038 (Dependency Injection Safety — captive dependency).

What changed

A new OrderReminderBackgroundService is registered as a singleton
hosted service alongside a brand-new IOrderEventEmitter registered as
scoped, all inside the same DI composition root:

// New background service that nudges customers about pending orders.
builder.Services.AddSingleton<OrderReminderBackgroundService>();
builder.Services.AddHostedService(sp =>
    sp.GetRequiredService<OrderReminderBackgroundService>());

// New scoped emitter so per-request correlation can flow into events.
builder.Services.AddScoped<IOrderEventEmitter, OrderEventEmitter>();

The new background service constructor takes IOrderEventEmitter
directly:

public OrderReminderBackgroundService(IOrderEventEmitter emitter)
{
    _emitter = emitter;
}

Why this is risky

  • A singleton resolves its dependencies once, at the moment the
    container builds it. A scoped service injected directly into a
    singleton becomes a captive dependency — the singleton holds the
    same scoped instance forever, across every request and every scope.
  • For IOrderEventEmitter, this means a per-request correlation id is
    frozen on first use and reused for every subsequent event — silently
    attributing every emitted event to the original request.
  • The bug is invisible in unit tests (which build a fresh container per
    test) and only manifests under real traffic, often weeks after deploy
    when a debugging engineer notices that "every error has the same
    trace id."

What GauntletCI catches

GCI0038 Dependency Injection Safety — the diff registers both an
AddSingleton<…> and an AddScoped<…> lifetime in the same file
(Program.cs), the canonical shape of a captive-dependency wiring
mistake.

How to fix it

  • Inject IServiceScopeFactory into the singleton instead, and create
    a scope per work item to resolve IOrderEventEmitter fresh each time.
  • Or promote the emitter to singleton if it is genuinely stateless.
  • Or convert the background service itself into a hosted-scope pattern
    (IServiceScopeFactory.CreateScope() inside ExecuteAsync).

…es a scoped dependency

See scenarios/17-captive-dependency/README.md for the expected verdict.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GauntletCI found the following issues:

These findings reference lines outside the PR diff, so they appear here instead of inline. Expand each entry for full evidence, rationale, and suggested action.

GCI0038 — Dependency Injection Safety (`src/OrderService/Program.cs`): Potential captive dependency: singleton may capture scoped service

GCI0038 — Dependency Injection Safety

Potential captive dependency: singleton may capture scoped service

Evidence:

src/OrderService/Program.cs: mixed lifetimes detected — builder.Services.AddSingleton();

⚠️ Why it matters: A singleton that depends on a scoped service will capture a stale instance, causing bugs that are hard to diagnose.

💡 Suggested action: Ensure singleton services only depend on other singletons, or use IServiceScopeFactory to create scopes explicitly.

Confidence: Medium | Severity: Warn

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: de24ab40b5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +25 to +28
builder.Services.AddSingleton<OrderReminderBackgroundService>();
builder.Services.AddHostedService(sp =>
sp.GetRequiredService<OrderReminderBackgroundService>());
builder.Services.AddScoped<IOrderEventEmitter, OrderEventEmitter>();
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid scoped dependency in singleton hosted service

OrderReminderBackgroundService is registered as a singleton hosted service while IOrderEventEmitter is registered as scoped, and the singleton takes that interface in its constructor. This is a captive dependency: with scope validation disabled (typical production), the scoped emitter is effectively promoted to app lifetime and can leak request-scoped state; with scope validation enabled, startup can fail when resolving the hosted service. Use IServiceScopeFactory inside the background service or make the lifetimes consistent.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant