Skip to content

fix: Create a single TaskBlockingListener per process for CaptureBlockingCalls - #5381

Merged
jamescrosswell merged 5 commits into
mainfrom
fix/issue-5378-blocking-listener-leak
Jul 16, 2026
Merged

fix: Create a single TaskBlockingListener per process for CaptureBlockingCalls#5381
jamescrosswell merged 5 commits into
mainfrom
fix/issue-5378-blocking-listener-leak

Conversation

@jamescrosswell

@jamescrosswell jamescrosswell commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a per-request resource leak in CaptureBlockingCalls (ASP.NET Core), reported in #5378.

SentryMiddleware implements IMiddleware and is registered with AddTransient<SentryMiddleware>(), so DI constructs a new instance per request. With CaptureBlockingCalls = true, its constructor created a new TaskBlockingListener (a System.Diagnostics.Tracing.EventListener) on every request and never disposed it:

_monitor = new BlockingMonitor(_getHub, _options);
_detectBlockingSyncCtx = new DetectBlockingSynchronizationContext(_monitor);
_listener = new TaskBlockingListener(_monitor); // registers itself globally, never disposed

Every EventListener registers itself in the runtime's process-global listener chain and enables TplEventSource (Verbose). So the chain grew by one per request and EventSource.DispatchToAllListeners walked the whole (ever-growing) chain on every task-wait event of every await in the process — not just inside requests.

The reporter observed, on effectively-idle services (only a 10s healthcheck): p95 of a trivial endpoint growing ~5 ms → ~150 ms over 5 days (~45k requests → ~45k leaked listeners), 96–97% of CPU inside EventSource.DispatchToAllListeners, and steadily growing background CPU and managed heap — all reset by a restart. Full credit to @gerardfontmora for the diagnosis.

Fix

  • Register BlockingMonitor and TaskBlockingListener as singletons, and have the transient middleware resolve (rather than construct) them, so exactly one TaskBlockingListener exists per process with constant overhead. They are only resolved when CaptureBlockingCalls is enabled, so disabling the feature has no cost.
  • The per-request DetectBlockingSynchronizationContext swap in InvokeAsync is unchanged — it carries per-request suppression state and is correctly created per request.
  • Drive-by: TaskBlockingListener.DefaultState was an expression-bodied property (=> new()) that allocated a fresh Lazy<> on every access instead of returning a single shared instance; changed to a static auto-property.

Regression test

Constructor_CaptureBlockingCalls_SharesSingleListenerAcrossInstances builds two middleware instances (simulating two requests) and asserts they share one Listener/Monitor. Verified it fails against the previous per-request construction and passes with this change.

Fixes #5378

…kingCalls

SentryMiddleware is registered as a transient IMiddleware, so DI constructs a
new instance per request. With CaptureBlockingCalls enabled, its constructor
created a new TaskBlockingListener (an EventListener) every request and never
disposed it. Each listener registered itself in the runtime's process-global
listener chain and enabled TplEventSource, so the chain grew unboundedly and
EventSource.DispatchToAllListeners walked it on every task-wait of every await
in the process - degrading latency, CPU, and managed heap linearly with the
total number of requests served.

Register BlockingMonitor and TaskBlockingListener as singletons and have the
transient middleware resolve (rather than construct) them, so exactly one
listener exists per process. The per-request DetectBlockingSynchronizationContext
swap is unchanged, since it carries per-request suppression state.

Also fix TaskBlockingListener.DefaultState, which was an expression-bodied
property allocating a fresh Lazy on every access instead of a single shared
instance.

Fixes #5378

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.28%. Comparing base (5e9a7c4) to head (6724a01).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/Sentry.AspNetCore/SentryMiddleware.cs 60.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5381      +/-   ##
==========================================
+ Coverage   74.25%   74.28%   +0.03%     
==========================================
  Files         509      509              
  Lines       18420    18427       +7     
  Branches     3606     3606              
==========================================
+ Hits        13677    13688      +11     
+ Misses       3869     3866       -3     
+ Partials      874      873       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/Sentry.AspNetCore/SentryMiddleware.cs Outdated
Comment thread src/Sentry.AspNetCore/SentryMiddleware.cs Outdated
Comment thread src/Sentry.AspNetCore/SentryMiddleware.cs Outdated
Comment thread src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs Outdated
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
Comment thread src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs Outdated
jamescrosswell and others added 2 commits July 14, 2026 15:28
Address review feedback: instead of manual factory lambdas, register the
services by type and let the container resolve their constructors. BlockingMonitor
takes (Func<IHub>, SentryOptions) - both already registered - and TaskBlocking
listener takes IBlockingMonitor, so an IBlockingMonitor -> BlockingMonitor forward
keeps them the same singleton instance.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-leak' into fix/issue-5378-blocking-listener-leak

# Conflicts:
#	src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs
Comment thread src/Sentry.AspNetCore/SentryWebHostBuilderExtensions.cs Outdated
Address review feedback: register the monitor as
AddSingleton<IBlockingMonitor, BlockingMonitor>() and have the middleware
resolve IBlockingMonitor (which is all DetectBlockingSynchronizationContext
needs) instead of the concrete type, removing the separate interface forward.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jamescrosswell
jamescrosswell marked this pull request as ready for review July 15, 2026 01:28
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Jul 15, 2026

@bruno-garcia bruno-garcia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, I'd add some integration tests too as a follow up

@jamescrosswell

Copy link
Copy Markdown
Collaborator Author

LGTM, I'd add some integration tests too as a follow up

Follow up issue:

This was referenced Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: medium PR risk score: medium

Projects

None yet

2 participants