Skip to content

fix(profiling): release the EventPipe session when the SDK shuts down - #5470

Draft
jamescrosswell wants to merge 1 commit into
mainfrom
fix/5418-profiler-session-dispose
Draft

fix(profiling): release the EventPipe session when the SDK shuts down#5470
jamescrosswell wants to merge 1 commit into
mainfrom
fix/5418-profiler-session-dispose

Conversation

@jamescrosswell

Copy link
Copy Markdown
Collaborator

SamplingTransactionProfilerFactory.Dispose() disposed the antecedent Task<SampleProfilerSession> rather than the session it wraps, so the EventPipeSession was never stopped.

Tracing it through, that line is the last of three defects sitting between "SDK shuts down" and "EventPipe session released" — fixing it alone would not have changed anything observable:

  1. SampleProfilerSession.Stop() skipped its own disposals on the happy path. _processing was built with TaskContinuationOptions.OnlyOnFaulted; when eventSource.Process() returns normally the continuation criteria aren't met, so the continuation transitions to Canceled and _processing.Wait() throws AggregateException(TaskCanceledException). EventPipeSession.Dispose() and TraceLogEventSource.Dispose() sat after that call and were never reached — the catch logged Error during sampler profiler session shutdown. and swallowed it. The continuation is now unconditional, and the disposals moved into a finally with a bounded drain wait so shutdown can't hang.

  2. Nothing ever called Dispose() on the factory. ProfilingIntegration was not IDisposable, so Hub never added it to _integrationsToCleanup, and the factory's only other reference is options.TransactionProfilerFactory, which nothing disposes. The integration is now IDisposable and disposes the factory — but only one it created itself, since TransactionProfilerFactory may have been supplied elsewhere.

  3. The reported bug. Dispose() now waits (bounded) for an in-flight startup and then disposes the session. It also cancels the WaitForFirstEventAsync wait, which could otherwise block indefinitely, and observes the startup task's exception if it failed.

Impact

Low, and narrower than the issue suggests. The session is created once and lives for the SDK's lifetime — SamplingTransactionProfiler.Stop() only clears _inProgress, it never touches the session — so the only disposal point is SDK shutdown, which is normally process shutdown, where the OS reclaims the handle and the TraceLog regardless. Nothing accumulates while an application runs.

Where it does bite is when the process outlives the SDK: repeated Init/Close cycles strand one EventPipeSession plus its TraceLog per cycle, and hosts that close Sentry but keep running never release the pipe.

The issue links this to the Windows Service memory growth in #3375. That link doesn't hold up — a service that initialises Sentry once would never reach any of these paths.

Notes

  • Defect 1 has a visible signature in diagnostic logs: Error during sampler profiler session shutdown. on every clean shutdown with profiling enabled. Worth asking anyone reporting profiler-related growth whether they see it.
  • Hub.Dispose() now does real work for profiling users where it previously did none. Both new waits are bounded at 2s so a wedged EventPipe session can't hang shutdown.
  • The existing Sentry.Profiling tests are largely Skip.If(TestEnvironment.IsGitHubActions), so the two ProfilingIntegration tests are plain [Fact]s that do run in CI; only the session-level test follows the existing skip pattern.

Fixes #5418

SamplingTransactionProfilerFactory.Dispose() disposed the antecedent
Task<SampleProfilerSession> rather than the session it wraps. Fixing that
alone changes nothing observable, because two further defects sit between
"SDK shuts down" and "EventPipe session released":

- SampleProfilerSession.Stop() built _processing as an OnlyOnFaulted
  continuation, which transitions to Canceled when Process() returns
  normally - so Wait() threw on every clean shutdown and the disposals
  after it were never reached.
- ProfilingIntegration was not IDisposable, so Hub never registered it for
  cleanup and the factory's Dispose() was only ever called by tests.

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

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 65.78947% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.74%. Comparing base (e669940) to head (948f8b5).

Files with missing lines Patch % Lines
src/Sentry.Profiling/SampleProfilerSession.cs 33.33% 6 Missing and 2 partials ⚠️
...ry.Profiling/SamplingTransactionProfilerFactory.cs 73.68% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5470      +/-   ##
==========================================
+ Coverage   74.71%   74.74%   +0.03%     
==========================================
  Files         513      513              
  Lines       18729    18758      +29     
  Branches     3663     3669       +6     
==========================================
+ Hits        13993    14021      +28     
+ Misses       3865     3858       -7     
- Partials      871      879       +8     

☔ 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.

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.

SamplingTransactionProfilerFactory.Dispose disposes the Task, not the SampleProfilerSession (resource leak)

1 participant