Skip to content

Dispose endpoint rate limiter on application shutdown - #69914

Open
SENESO wants to merge 2 commits into
dotnet:mainfrom
SENESO:fix/ratelimiting-dispose-66434
Open

SENESO wants to merge 2 commits into
dotnet:mainfrom
SENESO:fix/ratelimiting-dispose-66434

Conversation

@SENESO

@SENESO SENESO commented Oct 18, 2026

Copy link
Copy Markdown

Fixes #66434 (also closes #69061 as a duplicate).

RateLimitingMiddleware creates its own PartitionedRateLimiter<HttpContext> (_endpointLimiter) in CreateEndpointLimiter() but never disposes it. The limiter's internal timer infrastructure keeps the host rooted, so every WebApplicationFactory-based test class leaks roughly one host (~100MB). Verified with a WeakReference repro: the host is not collectable with UseRateLimiter before this change, and collectable after.

The fix registers the middleware-owned limiter for disposal on IHostApplicationLifetime.ApplicationStopped. Design notes:

  • Only _endpointLimiter is disposed — it is created inside the middleware, so the middleware is its sole owner. _globalLimiter comes from RateLimiterOptions.GlobalLimiter (user-provided) and is deliberately left alone: the user may share it across hosts or dispose it themselves.
  • GetService (not GetRequiredService) keeps this null-safe for non-host scenarios; the callback is a static lambda so the registration can't root the middleware instance.

Tests:

  • EndpointLimiterIsDisposedWhenApplicationStops — fails without the fix, passes with it.
  • GlobalLimiterIsNotDisposedWhenApplicationStops — documents the ownership decision.

Note: I couldn't execute the repo's own test suite in my environment (it needs the source-built targeting pack from a near-full repo build), so CI is the real gate here. The change is minimal, additive, and null-safe.

@SENESO
SENESO requested a review from BrennanConroy as a code owner October 18, 2026 07:01
@github-actions github-actions Bot added the area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares label Oct 18, 2026
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 18, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Thanks for your PR, @SENESO. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@SENESO

SENESO commented Oct 18, 2026

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

@MohamedAliTera

Copy link
Copy Markdown

Any update on this? 👀

@SENESO

SENESO commented Oct 25, 2026

Copy link
Copy Markdown
Author

Hi @MohamedAliTera, thanks for checking in. The fix is complete and all CI checks are green — it's currently waiting on a maintainer review. Hopefully it gets picked up soon.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service Bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Nov 2, 2026

This branch has not been deployed

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

Labels

area-middleware Includes: URL rewrite, redirect, response cache/compression, session, and other general middlewares community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun

Projects

None yet

2 participants